From d3f1a1a4001c952d2a3041c0da03b1ff8f4476c1 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Thu, 8 Oct 2026 11:59:01 +0100 Subject: [PATCH 1/7] Sync: split up openfn.yaml (#1567) * lexicon: remove trigger.enabled from spec * update handling of trigger.enabled * update version hash * changeset * remove log * update version util and fix tests * add notes to docs * remove .only * fix test * update test * fix integration test * first swing at refactor to split up openfn.yaml * order keys * fix ropey merge issues * add logic to checkout on git without tracking * Update tests for checkout file and flat openfn.yaml Project metadata now lives in .openfn/checkout.yaml, openfn.yaml holds workspace config (plus collections) as flat, sorted keys. Integration tests pin OPENFN_BRANCH=false so they don't pick up the kit repo's branch. Co-Authored-By: Claude Sonnet 5.5 * format * move collections to resources.yaml * Add prompt to reworked openfn yaml (#1577) * add util for checking if there's a valid checkout * add helpers for writing checkout metadata * refactoring * use new helpers in deploy * prompt user for checkout * add user prompt * simplify * fix tests * better fix * ignore checkout meta (#1578) * changeset * changesets and docs --------- Co-authored-by: Claude Sonnet 5.5 --- .changeset/eager-houses-call.md | 10 + .changeset/hip-actors-switch.md | 5 + .changeset/orange-lions-talk.md | 5 + .changeset/small-cases-buy.md | 5 + .changeset/true-dolls-stay.md | 6 + integration-tests/cli/.gitignore | 3 +- integration-tests/cli/src/run.ts | 2 + integration-tests/cli/test/deploy.test.ts | 5 +- integration-tests/cli/test/deploy.v2.test.ts | 36 ++- integration-tests/cli/test/project-v2.test.ts | 11 +- integration-tests/cli/test/sync.test.ts | 18 +- packages/cli/src/projects/checkout.ts | 68 +++- packages/cli/src/projects/clean.ts | 29 +- packages/cli/src/projects/deploy.ts | 63 +++- packages/cli/src/projects/diff.ts | 26 +- packages/cli/src/projects/ensure-checkout.ts | 125 ++++++++ packages/cli/src/projects/fetch.ts | 10 +- packages/cli/src/projects/list.ts | 13 +- packages/cli/src/projects/merge.ts | 14 +- packages/cli/src/projects/options.ts | 50 +++ packages/cli/src/projects/pull.ts | 14 +- packages/cli/src/projects/util.ts | 40 ++- packages/cli/src/projects/version.ts | 20 +- packages/cli/test/projects/checkout.test.ts | 90 ++++-- packages/cli/test/projects/clean.test.ts | 40 +++ packages/cli/test/projects/deploy.test.ts | 254 ++++++++++++++- .../cli/test/projects/ensure-checkout.test.ts | 300 ++++++++++++++++++ packages/cli/test/projects/list.test.ts | 26 ++ packages/cli/test/projects/util.test.ts | 121 ++++++- packages/lexicon/core.d.ts | 3 +- packages/project/src/Project.ts | 21 +- packages/project/src/Workspace.ts | 24 +- packages/project/src/index.ts | 4 + packages/project/src/merge/merge-project.ts | 13 +- packages/project/src/parse/from-fs.ts | 41 ++- packages/project/src/serialize/to-fs.ts | 24 +- packages/project/src/util/config.ts | 102 +++++- packages/project/src/util/project-diff.ts | 33 +- packages/project/src/util/resources.ts | 56 ++++ .../project/test/merge/merge-project.test.ts | 24 +- packages/project/test/parse/from-fs.test.ts | 81 ++++- packages/project/test/serialize/to-fs.test.ts | 154 +++++---- packages/project/test/util/config.test.ts | 162 +++++++--- .../project/test/util/project-diff.test.ts | 42 +++ packages/project/test/util/resources.test.ts | 65 ++++ 45 files changed, 1968 insertions(+), 290 deletions(-) create mode 100644 .changeset/eager-houses-call.md create mode 100644 .changeset/hip-actors-switch.md create mode 100644 .changeset/orange-lions-talk.md create mode 100644 .changeset/small-cases-buy.md create mode 100644 .changeset/true-dolls-stay.md create mode 100644 packages/cli/src/projects/ensure-checkout.ts create mode 100644 packages/cli/test/projects/ensure-checkout.test.ts create mode 100644 packages/project/test/util/resources.test.ts diff --git a/.changeset/eager-houses-call.md b/.changeset/eager-houses-call.md new file mode 100644 index 000000000..41929b61e --- /dev/null +++ b/.changeset/eager-houses-call.md @@ -0,0 +1,10 @@ +--- +'@openfn/project': minor +'@openfn/cli': minor +--- + +Restructure `openfn.yaml` to make it safer for merging on github. + +Any checkout state (basically anything under the project key) have been moved into a new `.openfn` folder, which should not be tracked. + +`openfn.yaml` now just contains workspace-wide configuration. diff --git a/.changeset/hip-actors-switch.md b/.changeset/hip-actors-switch.md new file mode 100644 index 000000000..33e091794 --- /dev/null +++ b/.changeset/hip-actors-switch.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': patch +--- + +Fix an issue where `project clean` can try to delete stuff even if nothing is checked out diff --git a/.changeset/orange-lions-talk.md b/.changeset/orange-lions-talk.md new file mode 100644 index 000000000..3a458c822 --- /dev/null +++ b/.changeset/orange-lions-talk.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': patch +--- + +Include collections and channels in sync diffs diff --git a/.changeset/small-cases-buy.md b/.changeset/small-cases-buy.md new file mode 100644 index 000000000..a05e27a3e --- /dev/null +++ b/.changeset/small-cases-buy.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': patch +--- + +When running commands with v2 sync, if no checkout metadata is detected, prompt the user diff --git a/.changeset/true-dolls-stay.md b/.changeset/true-dolls-stay.md new file mode 100644 index 000000000..3e537705b --- /dev/null +++ b/.changeset/true-dolls-stay.md @@ -0,0 +1,6 @@ +--- +'@openfn/project': patch +'@openfn/cli': patch +--- + +Track collections in resources.yaml diff --git a/integration-tests/cli/.gitignore b/integration-tests/cli/.gitignore index ffdcec42c..f3bb1a002 100644 --- a/integration-tests/cli/.gitignore +++ b/integration-tests/cli/.gitignore @@ -1 +1,2 @@ -repo \ No newline at end of file +repo +.state.json diff --git a/integration-tests/cli/src/run.ts b/integration-tests/cli/src/run.ts index fd00339e0..9735d1305 100644 --- a/integration-tests/cli/src/run.ts +++ b/integration-tests/cli/src/run.ts @@ -7,6 +7,8 @@ const getOptions = () => ({ env: { ...process.env, OPENFN_REPO_DIR: path.resolve('repo'), + // workspaces live inside the kit git repo: don't pick up its branch + OPENFN_BRANCH: 'false', }, }); diff --git a/integration-tests/cli/test/deploy.test.ts b/integration-tests/cli/test/deploy.test.ts index 302d02a68..7110cfde9 100644 --- a/integration-tests/cli/test/deploy.test.ts +++ b/integration-tests/cli/test/deploy.test.ts @@ -273,7 +273,10 @@ test.serial('redirect to v2 protocol if openfn.yaml is present', async (t) => { t.falsy(bootstrap.stderr); assertLog(t, extractLogs(bootstrap.stdout), /Detected openfn.yaml file/i); - const yaml = await fs.readFile(path.join(tmpDir, 'openfn.yaml'), 'utf8'); + const yaml = await fs.readFile( + path.join(tmpDir, '.openfn/checkout.yaml'), + 'utf8' + ); t.regex(yaml, new RegExp(`uuid\\: ${projectId}`)); const workflowYaml = await fs.readFile( diff --git a/integration-tests/cli/test/deploy.v2.test.ts b/integration-tests/cli/test/deploy.v2.test.ts index e7995e1d8..7e5139dda 100644 --- a/integration-tests/cli/test/deploy.v2.test.ts +++ b/integration-tests/cli/test/deploy.v2.test.ts @@ -40,7 +40,10 @@ test.serial('pull a project', async (t) => { assertLog(t, extractLogs(stdout), /Checked out project locally/i); - const yaml = await fs.readFile(path.resolve(tmpDir, 'openfn.yaml'), 'utf8'); + const yaml = await fs.readFile( + path.resolve(tmpDir, '.openfn/checkout.yaml'), + 'utf8' + ); t.regex(yaml, /uuid\: a/); t.regex(yaml, /id\: test-project/); }); @@ -56,7 +59,10 @@ test.serial('pull, change and re-deploy', async (t) => { t.falsy(pullResult.stderr); assertLog(t, extractLogs(pullResult.stdout), /Checked out project locally/i); - const yaml = await fs.readFile(path.resolve(tmpDir, 'openfn.yaml'), 'utf8'); + const yaml = await fs.readFile( + path.resolve(tmpDir, '.openfn/checkout.yaml'), + 'utf8' + ); t.regex(yaml, /id\: test-project/); // modify expression to trigger a change @@ -520,7 +526,7 @@ test.serial( ); test.serial( - 'deploy collections: add a collection via openfn.yaml', + 'deploy collections: add a collection via resources.yaml', async (t) => { const projectId = 'gggggggg'; server.addProject(makeProject(projectId) as any); @@ -538,13 +544,13 @@ test.serial( /Checked out project locally/i ); - const openfnPath = path.resolve(tmpDir, 'openfn.yaml'); - const before: any = yamlToJson(await fs.readFile(openfnPath, 'utf8')); - t.falsy(before.project.collections); + const resourcesPath = path.resolve(tmpDir, 'resources.yaml'); + const before: any = yamlToJson(await fs.readFile(resourcesPath, 'utf8')); + t.deepEqual(before.collections, {}); // add a collection by hand - no workflow files are touched - before.project.collections = ['my-collection']; - await fs.writeFile(openfnPath, jsonToYaml(before)); + before.collections = { 'my-collection': {} }; + await fs.writeFile(resourcesPath, jsonToYaml(before)); const { stdout, stderr } = await run( `openfn project deploy --no-confirm --log-json -l debug` @@ -623,7 +629,7 @@ workflows: }); test.serial( - 'deploy collections: remove a collection via openfn.yaml', + 'deploy collections: remove a collection via resources.yaml', async (t) => { const projectId = 'hhhhhhhh'; server.addProject({ @@ -633,7 +639,7 @@ test.serial( t.is(server.state.projects[projectId].collections.length, 1); - // pull the project - openfn.yaml should list the fetched collection + // pull the project - resources.yaml should list the fetched collection const pullResult = await run( `openfn project pull ${projectId} --log-json -l debug` ); @@ -644,13 +650,13 @@ test.serial( /Checked out project locally/i ); - const openfnPath = path.resolve(tmpDir, 'openfn.yaml'); - const before: any = yamlToJson(await fs.readFile(openfnPath, 'utf8')); - t.deepEqual(before.project.collections, ['my-collection']); + const resourcesPath = path.resolve(tmpDir, 'resources.yaml'); + const before: any = yamlToJson(await fs.readFile(resourcesPath, 'utf8')); + t.deepEqual(before.collections, { 'my-collection': {} }); // remove the collection by hand - no workflow files are touched - before.project.collections = []; - await fs.writeFile(openfnPath, jsonToYaml(before)); + before.collections = {}; + await fs.writeFile(resourcesPath, jsonToYaml(before)); const { stdout, stderr } = await run( `openfn project deploy --no-confirm --log-json -l debug` diff --git a/integration-tests/cli/test/project-v2.test.ts b/integration-tests/cli/test/project-v2.test.ts index 8beaf99f8..5bd26a5d9 100644 --- a/integration-tests/cli/test/project-v2.test.ts +++ b/integration-tests/cli/test/project-v2.test.ts @@ -282,13 +282,12 @@ steps: ); // add the credential map to the yaml + await writeFile(`${TMP_DIR}/openfn.yaml`, `credentials: creds.yaml`); + + // point the checked out project at the mock server await writeFile( - `${TMP_DIR}/openfn.yaml`, - ` -project: - endpoint: http://localhost:1234 -workspace: - credentials: creds.yaml` + `${TMP_DIR}/.openfn/checkout.yaml`, + `endpoint: http://localhost:1234` ); // write the credential map diff --git a/integration-tests/cli/test/sync.test.ts b/integration-tests/cli/test/sync.test.ts index 38179a93e..8291a6a64 100644 --- a/integration-tests/cli/test/sync.test.ts +++ b/integration-tests/cli/test/sync.test.ts @@ -160,9 +160,9 @@ test('pull a new project', async (t) => { t.regex(proj_yaml, /id: patients/); t.regex(proj_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); - const openfn_yaml = read('openfn.yaml'); - t.regex(openfn_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); - t.regex(openfn_yaml, new RegExp(`endpoint: ${endpoint}`)); + const checkout_yaml = read('.openfn/checkout.yaml'); + t.regex(checkout_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); + t.regex(checkout_yaml, new RegExp(`endpoint: ${endpoint}`)); const job = read('workflows/workflow/job.js'); t.is(job, 'fn()'); @@ -188,9 +188,9 @@ test('pull a new project with an alias', async (t) => { t.regex(proj_yaml, /id: patients/); t.regex(proj_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); - const openfn_yaml = read('openfn.yaml'); - t.regex(openfn_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); - t.regex(openfn_yaml, new RegExp(`endpoint: ${endpoint}`)); + const checkout_yaml = read('.openfn/checkout.yaml'); + t.regex(checkout_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); + t.regex(checkout_yaml, new RegExp(`endpoint: ${endpoint}`)); const job = read('workflows/workflow/job.js'); t.is(job, 'fn()'); @@ -231,9 +231,9 @@ test('pull an update to project', async (t) => { t.regex(proj_yaml, /fn\(x\)/); t.regex(proj_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); - const openfn_yaml = read('openfn.yaml'); - t.regex(openfn_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); - t.regex(openfn_yaml, new RegExp(`endpoint: ${endpoint}`)); + const checkout_yaml = read('.openfn/checkout.yaml'); + t.regex(checkout_yaml, new RegExp(`uuid: ${project.openfn.uuid}`)); + t.regex(checkout_yaml, new RegExp(`endpoint: ${endpoint}`)); const job_updated = read('workflows/workflow/job.js'); t.is(job_updated, 'fn()'); diff --git a/packages/cli/src/projects/checkout.ts b/packages/cli/src/projects/checkout.ts index 08904f14d..d004773cf 100644 --- a/packages/cli/src/projects/checkout.ts +++ b/packages/cli/src/projects/checkout.ts @@ -1,5 +1,6 @@ import yargs from 'yargs'; import Project, { Workspace } from '@openfn/project'; +import type l from '@openfn/lexicon'; import path from 'path'; import fs from 'fs'; import { rimraf } from 'rimraf'; @@ -10,7 +11,11 @@ import * as o from '../options'; import * as po from './options'; import type { Opts } from './options'; -import { tidyWorkflowDir, updateForkedFrom } from './util'; +import { + ensureCheckoutIgnored, + tidyWorkflowDir, + updateForkedFrom, +} from './util'; import { createProjectCredentials } from './credentials-helpers'; import abort from '../util/abort'; @@ -23,9 +28,19 @@ export type CheckoutOptions = Pick< | 'clean' | 'force' | 'createCredentials' + | 'branch' + | 'track' >; -const options = [o.log, po.workspace, po.clean, o.force, po.creds]; +const options = [ + o.log, + po.workspace, + po.branch, + po.track, + po.clean, + o.force, + po.creds, +]; const command: yargs.CommandModule = { command: 'checkout ', @@ -43,7 +58,9 @@ export default command; export const handler = async (options: CheckoutOptions, logger?: Logger) => { const projectIdentifier = options.project!; const workspacePath = options.workspace ?? process.cwd(); - const workspace = new Workspace(workspacePath, logger); + const workspace = new Workspace(workspacePath, logger, true, { + branch: options.branch, + }); // get the config // TODO: try to retain the endpoint for the projects @@ -109,18 +126,52 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { } // Check whether the checked out project has diverged from its forked from versions + // On a git branch, the checkout file binds the branch to a project. + // Checking out some other project is an ad-hoc checkout: expand its files, + // but keep the branch tracking the original project + const tracked = workspace.activeProject; + const isAdHoc = Boolean( + options.branch && + !options.track && + tracked && + !isSameProject(tracked, switchProject) + ); + if (isAdHoc) { + const trackedName = tracked!.alias ?? tracked!.id ?? tracked!.uuid; + logger?.warn( + `Branch ${options.branch} tracks project ${trackedName}. Expanding files from ${switchProject.alias} without changing tracking: deploy will still target ${trackedName}` + ); + logger?.warn( + `Pass --track to make ${options.branch} track ${switchProject.alias} instead` + ); + } + // delete workflow dir before expanding project if (options.clean) { await rimraf(workspace.workflowsPath); } else { - await tidyWorkflowDir(localProject, switchProject, false, workspacePath); + await tidyWorkflowDir( + localProject, + switchProject, + false, + workspacePath, + options.branch + ); } // write the forked from map updateForkedFrom(switchProject); // expand project into directory - const files: any = switchProject.serialize('fs'); + const files: any = switchProject.serialize('fs', { + branch: options.branch, + }); + if (isAdHoc) { + // Don't touch the workspace or checkout metadata + delete files[switchProject.generateConfig().path]; + delete files[switchProject.generateCheckout(options.branch).path]; + } + await ensureCheckoutIgnored(workspacePath); for (const f in files) { if (files[f]) { fs.mkdirSync(path.join(workspacePath, path.dirname(f)), { @@ -143,6 +194,13 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { logger?.success(`Expanded project to ${workspacePath}`); }; +const isSameProject = (tracked: l.ProjectMeta, project: Project) => { + if (tracked.uuid && project.openfn?.uuid) { + return tracked.uuid === project.openfn.uuid; + } + return tracked.id === project.id; +}; + // This function will tell us if the active/checked out project // has any changes compared to the tracked state file // It implies that changes will be lost on checkout diff --git a/packages/cli/src/projects/clean.ts b/packages/cli/src/projects/clean.ts index d919c53c3..9c9b63f97 100644 --- a/packages/cli/src/projects/clean.ts +++ b/packages/cli/src/projects/clean.ts @@ -4,6 +4,7 @@ import { rimraf } from 'rimraf'; import { build, ensure } from '../util/command-builders'; import { handler as checkout } from './checkout'; +import ensureCheckout from './ensure-checkout'; import type { Logger } from '../util/logger'; import * as o from '../options'; import * as po from './options'; @@ -12,10 +13,10 @@ import type { Opts } from './options'; export type CleanOptions = Pick< Opts, - 'command' | 'workspace' | 'log' | 'confirm' | 'force' + 'command' | 'workspace' | 'log' | 'confirm' | 'force' | 'branch' >; -const options = [o.log, o.confirm, o.force, po.workspace]; +const options = [o.log, o.confirm, o.force, po.workspace, po.branch]; const command: yargs.CommandModule = { command: 'clean', @@ -29,7 +30,22 @@ export default command; export const handler = async (options: CleanOptions, logger: Logger) => { const workspacePath = options.workspace ?? process.cwd(); - const workspace = new Workspace(workspacePath, logger); + + // Find out what's checked out before deleting anything + const workspace = await ensureCheckout( + new Workspace(workspacePath, logger, true, { + branch: options.branch, + }), + logger + ); + + // This has to be checked before deleting anything + const activeProject = workspace.activeProject; + if (!activeProject) { + throw new Error( + 'No active project found in workspace. Run `project pull` first.' + ); + } const skip = options.force || options.confirm === false; const doIt = await logger.confirm( @@ -42,13 +58,6 @@ export const handler = async (options: CleanOptions, logger: Logger) => { await rimraf(workspace.workflowsPath); - const activeProject = workspace.activeProject; - if (!activeProject) { - throw new Error( - 'No active project found in workspace. Run `project pull` first.' - ); - } - const projectId = String(activeProject.uuid ?? (activeProject as any).id); await checkout({ ...options, project: projectId, force: true }, logger); }; diff --git a/packages/cli/src/projects/deploy.ts b/packages/cli/src/projects/deploy.ts index ef0361c8b..88c4fd8f3 100644 --- a/packages/cli/src/projects/deploy.ts +++ b/packages/cli/src/projects/deploy.ts @@ -1,12 +1,13 @@ import yargs from 'yargs'; import Project, { MergeProjectOptions, + RESOURCES_FILE, toResourceChannels, versionsEqual, Workspace, } from '@openfn/project'; import { isEqual } from 'lodash-es'; -import { writeFile } from 'node:fs/promises'; +import { mkdir, readFile, writeFile } from 'node:fs/promises'; import path from 'node:path'; import * as o from '../options'; @@ -17,12 +18,13 @@ import { fetchProject, serialize, getSerializePath, - updateForkedFrom, + writeCheckoutFile, findLocallyChangedWorkflows, AuthOptions, } from './util'; import { build, ensure } from '../util/command-builders'; import { printRichDiff } from './diff'; +import ensureCheckout from './ensure-checkout'; import { getCredentialsVisitor, remapCredentials } from './credentials-helpers'; import type { Provisioner } from '@openfn/lexicon/lightning'; @@ -43,17 +45,16 @@ export type DeployOptions = Pick< | 'logJson' | 'confirm' > & { - // CLI positional args, rather than options - project?: string; - target?: string; - + project?: string; // this is a CLI positional arg, not an option alias?: string; + branch?: string | false; checkout?: boolean; credentials?: CredentialsStrategy; dryRun?: boolean; jsonDiff?: boolean; name?: string; new?: boolean; + target?: string; workflow?: string[]; workspace?: string; }; @@ -62,6 +63,7 @@ const options = [ // local options o2.env, o2.workspace, + o2.branch, o2.dryRun, o2.new, o2.name, @@ -152,7 +154,11 @@ export const hasRemoteDiverged = ( return diverged; }; +// A missing collections key (local.collections undefined) means collections +// aren't managed locally, so there's nothing to change export const collectionsChanged = (local: Project, remote: Project) => { + if (!local.collections) return false; + const names = (project: Project) => new Set((project.collections ?? []).map((c) => c.name)); @@ -164,11 +170,13 @@ export const collectionsChanged = (local: Project, remote: Project) => { }; // Collections dropped from the merged project (ie, removed from -// openfn.yaml) need an explicit delete entry in the deploy payload +// resources.yaml) need an explicit delete entry in the deploy payload export const deletedCollections = ( merged: Project, remote: Project ): Provisioner.Collection[] => { + if (!merged.collections) return []; + const keptNames = new Set((merged.collections ?? []).map((c) => c.name)); return (remote.collections ?? []) .filter((c) => !keptNames.has(c.name) && c.uuid) @@ -393,7 +401,16 @@ export async function handler(options: DeployOptions, logger: Logger) { logger.debug('Reading checked-out project from workspace'); // TODO this is the hard way to load the local alias // We need track alias in openfn.yaml to make this easier (and tracked in from fs) - ws = new Workspace(options.workspace || '.'); + ws = new Workspace(options.workspace || '.', undefined, true, { + branch: options.branch, + }); + + // We only need to know what's checked out if we're going to write the + // checkout back after deploying, and if the user hasn't told us which + // project to deploy to + if (options.checkout !== false && !targetIdentifier) { + ws = await ensureCheckout(ws, logger); + } const active = ws.getTrackedProject(); @@ -404,6 +421,7 @@ export async function handler(options: DeployOptions, logger: Logger) { localProject = await Project.from('fs', { root: options.workspace || '.', alias, + branch: options.branch, name: options.name, }); } @@ -425,7 +443,9 @@ export async function handler(options: DeployOptions, logger: Logger) { localProject.alias = alias ?? null; } } else { - ws ??= new Workspace(options.workspace || '.'); + ws ??= new Workspace(options.workspace || '.', undefined, true, { + branch: options.branch, + }); tracker = ws.get(targetIdentifier ?? localProject.uuid!); // A project loaded from a file already knows which remote it belongs @@ -605,16 +625,29 @@ export async function handler(options: DeployOptions, logger: Logger) { ); if (options.checkout !== false) { - updateForkedFrom(finalProject); - const configData = finalProject.generateConfig(); + const workspacePath = options.workspace ?? process.cwd(); - // Write the updated openfn.yaml + // Write the updated openfn.yaml and checkout file // TODO: allow us to suppress writing this stuff // (useful if posting from spec) - await writeFile( - path.resolve(options.workspace ?? process.cwd(), configData.path), - configData.content + const configData = finalProject.generateConfig(); + const configPath = path.resolve(workspacePath, configData.path); + await mkdir(path.dirname(configPath), { recursive: true }); + await writeFile(configPath, configData.content); + await writeCheckoutFile(workspacePath, finalProject, options.branch); + + // Sync collections into resources.yaml (anything else in there is left alone) + const resourcesPath = path.resolve(workspacePath, RESOURCES_FILE); + const existingResources = await readFile(resourcesPath, 'utf8').catch( + (e) => { + if (e.code !== 'ENOENT') throw e; + return undefined; + } ); + const resources = finalProject.generateResources(existingResources); + if (resources.content) { + await writeFile(resourcesPath, resources.content); + } // TODO if this was marked as new, we probably need to ensure a unique alias here const finalOutputPath = getSerializePath( diff --git a/packages/cli/src/projects/diff.ts b/packages/cli/src/projects/diff.ts index 7b72eb144..af32f8d25 100644 --- a/packages/cli/src/projects/diff.ts +++ b/packages/cli/src/projects/diff.ts @@ -1,6 +1,6 @@ import c from 'chalk'; import Project, { generateStepDiff, generateEdgeDiff } from '@openfn/project'; -import type { ChannelDiff } from '@openfn/project'; +import type { ChannelDiff, CollectionDiff } from '@openfn/project'; import type { StepChange, EdgeChange } from '@openfn/project'; import type { Logger } from '../util/logger'; @@ -81,8 +81,18 @@ const printChannelDiff = (channels: ChannelDiff[], logger: Logger) => { logger.break(); }; -// TODO need to include collection diffs -// https://github.com/OpenFn/kit/issues/1524 +const printCollectionDiff = (collections: CollectionDiff[], logger: Logger) => { + logger.always('Collections:'); + for (const { name, type } of collections) { + if (type === 'added') { + logger.always(c.green(` ${name}: added`)); + } else { + logger.always(c.red(` ${name}: removed`)); + } + } + logger.break(); +}; + export const printRichDiff = ( local: Project, remote: Project, @@ -93,7 +103,11 @@ export const printRichDiff = ( local, locallyChangedWorkflows ); - if (diffs.length === 0 && resources.channels.length === 0) { + if ( + diffs.length === 0 && + resources.channels.length === 0 && + resources.collections.length === 0 + ) { logger.info('No workflow changes detected'); return diffs; } @@ -140,5 +154,9 @@ export const printRichDiff = ( printChannelDiff(resources.channels, logger); } + if (resources.collections.length > 0) { + printCollectionDiff(resources.collections, logger); + } + return diffs; }; diff --git a/packages/cli/src/projects/ensure-checkout.ts b/packages/cli/src/projects/ensure-checkout.ts new file mode 100644 index 000000000..c14327665 --- /dev/null +++ b/packages/cli/src/projects/ensure-checkout.ts @@ -0,0 +1,125 @@ +import { select } from '@inquirer/prompts'; +import { Workspace, hasCheckoutMeta } from '@openfn/project'; +import type Project from '@openfn/project'; + +import abort from '../util/abort'; +import type { Logger } from '../util/logger'; +import { writeCheckoutFile } from './util'; + +// Resolves to the chosen project, or null if the user skipped the question +type Prompt = ( + projects: Project[], + defaultProject: Project +) => Promise; + +type EnsureCheckoutOptions = { + // Can we ask the user questions? Defaults to whether stdin is a terminal + interactive?: boolean; + prompt?: Prompt; + // If false, carry on without a checkout when we can't ask the user, rather + // than aborting (for commands that can work without one) + required?: boolean; +}; + +const name = (project: Project) => project.alias ?? project.id; + +// eg `staging (my-project | 1234-abcd)`. Either id may be missing +export const describeProject = (project: Project) => { + const ids = [project.id, project.openfn?.uuid].filter(Boolean).join(' | '); + return `${project.alias || '(no alias)'}${ids ? ` (${ids})` : ''}`; +}; + +const promptForProject: Prompt = (projects, defaultProject) => + select({ + message: + 'No checked out project found. Select the project which is currently tracked.', + choices: [ + ...projects.map((project) => ({ + name: describeProject(project), + value: project as Project | null, + description: project.openfn?.endpoint, + })), + { + name: 'Skip checkout', + value: null, + description: 'Continue without setting the checked out project', + }, + ], + default: defaultProject, + }); + +// Guess which project is checked out: one with an alias matching the git +// branch (or main, if there's no branch), else just the first. +// Pass undefined, not null or false, if there is no branch +export const getDefaultProject = (projects: Project[], branch = 'main') => + projects.find((p) => p.alias === branch) ?? projects[0]; + +/** + * Make sure the workspace knows which project is checked out. + * + * If there's no checkout metadata (say, after a fresh git clone, since it's + * not tracked in git), work out which project the files belong to and set + * up the checkout file. The files themselves are not touched. + * + * Returns a workspace that reflects the checkout. + */ +export default async ( + workspace: Workspace, + logger: Logger, + { + interactive = Boolean(process.stdin.isTTY), + prompt = promptForProject, + required = true, + }: EnsureCheckoutOptions = {} +) => { + if (!workspace.valid || hasCheckoutMeta(workspace.root, workspace.branch)) { + return workspace; + } + + const projects = workspace.list(); + if (!projects.length) { + // Nothing to choose from: let the command report that in its own way + return workspace; + } + + if (projects.length > 1 && !interactive && !required) { + logger.debug('No checked out project found: carrying on without one'); + return workspace; + } + + let project: Project | null = null; + if (projects.length === 1) { + project = projects[0]; + logger.info(`No checked out project found: using ${name(project)}`); + } else if (interactive) { + logger.info( + 'Failed to find a checkout.yaml file, which describes which project is currently checked out. This usually happens when pulling a project from git.' + ); + logger.info( + "No problem - just pick which project is checked out. This won't affect your working tree, it'll just update local metadata and tell the CLI which remote project instance to track\n." + ); + project = await prompt( + projects, + getDefaultProject(projects, workspace.branch || undefined) + ); + } else { + abort(logger, 'No checked out project found', { + details: `This workspace tracks several projects (${projects + .map(name) + .join(', ')}) and it's not known which one is checked out`, + fix: 'Run this command in an interactive terminal to choose one', + }); + } + + if (!project) { + logger.info('Skipped: no checked out project has been set'); + return workspace; + } + + await writeCheckoutFile(workspace.root, project, workspace.branch); + logger.success(`Set the checked out project to ${name(project)}`); + + return new Workspace(workspace.root, logger, true, { + branch: workspace.branch, + }); +}; diff --git a/packages/cli/src/projects/fetch.ts b/packages/cli/src/projects/fetch.ts index e87f0b3fa..0808cfe9e 100644 --- a/packages/cli/src/projects/fetch.ts +++ b/packages/cli/src/projects/fetch.ts @@ -31,6 +31,7 @@ export type FetchOptions = Pick< | 'outputPath' | 'project' | 'workspace' + | 'branch' >; const options = [ @@ -47,6 +48,7 @@ const options = [ po.outputPath, po.env, po.workspace, + po.branch, po.format, ]; @@ -75,7 +77,9 @@ const fetchV1 = async (options: FetchOptions, logger: Logger) => { const workspacePath = options.workspace ?? process.cwd(); logger.debug('Using workspace at', workspacePath); - const workspace = new Workspace(workspacePath, logger, false); + const workspace = new Workspace(workspacePath, logger, false, { + branch: options.branch, + }); // TODO we may need to resolve an alias to a UUID and endpoint const localProject = workspace.get(options.project!); if (localProject) { @@ -123,7 +127,9 @@ export const fetchV2 = async (options: FetchOptions, logger: Logger) => { const workspacePath = options.workspace ?? process.cwd(); logger.debug('Using workspace at', workspacePath); - const workspace = new Workspace(workspacePath, logger, false); + const workspace = new Workspace(workspacePath, logger, false, { + branch: options.branch, + }); const { outputPath } = options; const remoteProject = await fetchRemoteProject(workspace, options, logger); diff --git a/packages/cli/src/projects/list.ts b/packages/cli/src/projects/list.ts index 932ad182e..2b0576f88 100644 --- a/packages/cli/src/projects/list.ts +++ b/packages/cli/src/projects/list.ts @@ -8,10 +8,11 @@ import * as po from './options'; import type { Opts } from './options'; import abort from '../util/abort'; +import ensureCheckout from './ensure-checkout'; -export type ProjectListOptions = Pick; +export type ProjectListOptions = Pick; -const options = [o.log, po.workspace]; +const options = [o.log, po.workspace, po.branch]; const command: yargs.CommandModule = { command: 'list [project-path]', @@ -28,7 +29,9 @@ export const handler = async (options: ProjectListOptions, logger: Logger) => { logger.info(' ', options.workspace); logger.break(); - const workspace = new Workspace(options.workspace!); + let workspace = new Workspace(options.workspace!, undefined, true, { + branch: options.branch, + }); if (!workspace.valid) { // TODO how can we be more helpful here? @@ -40,6 +43,10 @@ export const handler = async (options: ProjectListOptions, logger: Logger) => { }); } + // Listing works fine without a checkout: it just can't mark the active + // project, so there's no need to abort if we can't ask which it is + workspace = await ensureCheckout(workspace, logger, { required: false }); + logger.always(`Available openfn projects\n\n${workspace .list() .map((p) => describeProject(p, p === workspace.getTrackedProject())) diff --git a/packages/cli/src/projects/merge.ts b/packages/cli/src/projects/merge.ts index d036cff8d..08634ce06 100644 --- a/packages/cli/src/projects/merge.ts +++ b/packages/cli/src/projects/merge.ts @@ -10,6 +10,7 @@ import * as po from './options'; import type { Opts } from './options'; import { handler as checkout } from './checkout'; +import ensureCheckout from './ensure-checkout'; export type MergeOptions = Required< Pick< @@ -17,13 +18,16 @@ export type MergeOptions = Required< 'command' | 'project' | 'workspace' | 'removeUnmapped' | 'workflowMappings' > > & - Pick & { base?: string }; + Pick & { + base?: string; + }; const options = [ po.removeUnmapped, po.workflowMappings, po.workflow, po.workspace, + po.branch, o.log, // custom output because we don't want defaults or anything { @@ -60,11 +64,16 @@ export default command; export const handler = async (options: MergeOptions, logger: Logger) => { const workspacePath = options.workspace; - const workspace = new Workspace(workspacePath); + let workspace = new Workspace(workspacePath, undefined, true, { + branch: options.branch, + }); if (!workspace.valid) { logger.error('Command was run in an invalid openfn workspace'); return; } + if (!options.base) { + workspace = await ensureCheckout(workspace, logger); + } let targetProject: Project; if (options.base) { @@ -169,6 +178,7 @@ export const handler = async (options: MergeOptions, logger: Logger) => { await checkout( { workspace: workspacePath, + branch: options.branch, project: options.outputPath ? finalPath : final.id, log: options.log, // after the merge, we have to force the output to be checked out, ignoring divergence diff --git a/packages/cli/src/projects/options.ts b/packages/cli/src/projects/options.ts index 759fe5353..1b4f8cb9e 100644 --- a/packages/cli/src/projects/options.ts +++ b/packages/cli/src/projects/options.ts @@ -1,3 +1,4 @@ +import { execSync } from 'node:child_process'; import resolvePath from '../util/resolve-path'; import { Opts as BaseOpts, CLIOption } from '../options'; import getCLIOptionObject from '../util/get-cli-option-object'; @@ -20,6 +21,8 @@ export type Opts = BaseOpts & { format?: 'yaml' | 'json' | 'state'; clean?: boolean; createCredentials?: boolean; + branch?: string | false; + track?: boolean; checkout?: boolean; }; @@ -49,6 +52,15 @@ export const clean: CLIOption = { }, }; +export const track: CLIOption = { + name: 'track', + yargs: { + boolean: true, + description: + 'When on a git branch, make this branch track the checked out project (by default, checking out a different project only expands its files)', + }, +}; + export const checkout: CLIOption = { name: 'checkout', yargs: { @@ -164,6 +176,44 @@ export const workspace: CLIOption = { }, }; +// Returns the current git branch, or false if not on a branch +// (not a git repo, git not installed, detached HEAD) +const detectBranch = (cwd: string) => { + try { + const branch = execSync('git symbolic-ref --short -q HEAD', { + cwd, + stdio: ['ignore', 'pipe', 'ignore'], + }) + .toString() + .trim(); + return branch || false; + } catch (e) { + return false; + } +}; + +// --branch is used to force the CLI to load checkout metadata from a different +// path in the .openfn folder. It is not likely to be used by users. +export const branch: CLIOption = { + name: 'branch', + yargs: { + hidden: true, + description: + 'The git branch to track the checked out project against. Detected automatically if not set. Pass --no-branch to ignore git.', + }, + ensure: (opts: any) => { + let value = opts.branch ?? process.env.OPENFN_BRANCH; + if (typeof value === 'string' && /^(false|null)?$/.test(value)) { + value = false; + } + if (value === undefined || value === true) { + const cwd = opts.workspace ?? process.env.OPENFN_WORKSPACE ?? '.'; + value = detectBranch(resolvePath(cwd)); + } + opts.branch = value === false ? false : String(value); + }, +}; + const newProject: CLIOption = { name: 'new', yargs: { diff --git a/packages/cli/src/projects/pull.ts b/packages/cli/src/projects/pull.ts index 5de9bf09c..06d183ef2 100644 --- a/packages/cli/src/projects/pull.ts +++ b/packages/cli/src/projects/pull.ts @@ -4,6 +4,7 @@ import { Workspace } from '@openfn/project'; import { build, ensure, override } from '../util/command-builders'; import { handler as fetch } from './fetch'; import { handler as checkout } from './checkout'; +import ensureCheckout from './ensure-checkout'; import * as o from '../options'; import * as o2 from './options'; @@ -25,6 +26,7 @@ export type PullOptions = Pick< | 'snapshots' | 'force' | 'createCredentials' + | 'branch' >; const options = [ @@ -33,6 +35,7 @@ const options = [ o2.alias, o2.env, o2.workspace, + o2.branch, o2.creds, // general options @@ -65,7 +68,7 @@ export const command: yargs.CommandModule = { export async function handler(options: PullOptions, logger: Logger) { options.workspace ??= process.cwd(); - ensureProjectId(options, logger); + await ensureProjectId(options, logger); await fetch(options, logger); logger.info(`Downloaded latest project version`); @@ -74,12 +77,17 @@ export async function handler(options: PullOptions, logger: Logger) { logger.success(`Checked out project locally`); } -const ensureProjectId = (options: any, logger?: Logger) => { +const ensureProjectId = async (options: any, logger: Logger) => { if (!options.project) { logger?.debug( 'No project ID specified: looking up checked out project in Workspace' ); - const ws = new Workspace(options.workspace); + const ws = await ensureCheckout( + new Workspace(options.workspace, undefined, true, { + branch: options.branch, + }), + logger + ); if (ws.activeProject) { options.project = ws.activeProject.uuid; logger?.info( diff --git a/packages/cli/src/projects/util.ts b/packages/cli/src/projects/util.ts index 64d5c7714..b77d7dd97 100644 --- a/packages/cli/src/projects/util.ts +++ b/packages/cli/src/projects/util.ts @@ -215,14 +215,15 @@ export async function tidyWorkflowDir( currentProject: Project | undefined, incomingProject: Project | undefined, dryRun = false, - dirPath = '.' + dirPath = '.', + branch?: string | false ) { if (!currentProject || !incomingProject) { return []; } - const currentFiles = currentProject.serialize('fs'); - const newFiles = incomingProject.serialize('fs'); + const currentFiles = currentProject.serialize('fs', { branch }); + const newFiles = incomingProject.serialize('fs', { branch }); const toRemove: string[] = []; // any files not in the new list should be removed @@ -259,6 +260,39 @@ export const updateForkedFrom = (proj: Project) => { return proj; }; +// The checkout state in .openfn is local to each machine, so it shouldn't be +// committed. Make git ignore everything in there. +// Set OPENFN_IGNORE_CHECKOUT_META=true to skip this +export const ensureCheckoutIgnored = async (workspacePath: string) => { + if (process.env.OPENFN_IGNORE_CHECKOUT_META === 'true') { + return; + } + + const filePath = path.resolve(workspacePath, '.openfn', '.gitignore'); + if (!fs.existsSync(filePath)) { + await mkdir(path.dirname(filePath), { recursive: true }); + await writeFile(filePath, '*\n'); + } +}; + +// Set up the checkout file for a project without touching anything else +// (use this when the workflows are already on disk, like after a git clone) +export const writeCheckoutFile = async ( + workspacePath: string, + project: Project, + branch?: string | false | null +) => { + updateForkedFrom(project); + await ensureCheckoutIgnored(workspacePath); + + const { path: checkoutPath, content } = project.generateCheckout(branch); + const filePath = path.resolve(workspacePath, checkoutPath); + await mkdir(path.dirname(filePath), { recursive: true }); + await writeFile(filePath, content); + + return filePath; +}; + // Compare a project to its version hashed when forked // This tells us whether the project was edited since it was created export const findLocallyChangedWorkflows = async ( diff --git a/packages/cli/src/projects/version.ts b/packages/cli/src/projects/version.ts index 46237d791..625c86ed5 100644 --- a/packages/cli/src/projects/version.ts +++ b/packages/cli/src/projects/version.ts @@ -12,11 +12,23 @@ import type { Opts } from './options'; export type VersionOptions = Required< Pick< Opts, - 'command' | 'workflow' | 'workspace' | 'workflowMappings' | 'json' | 'log' + | 'command' + | 'workflow' + | 'workspace' + | 'workflowMappings' + | 'json' + | 'log' + | 'branch' > >; -const options = [o.workflow, po.workspace, po.workflowMappings, o.log]; +const options = [ + o.workflow, + o.log, + po.workspace, + po.branch, + po.workflowMappings, +]; const command: yargs.CommandModule = { command: 'version [workflow]', @@ -95,7 +107,9 @@ export const handler = async (options: VersionOptions, logger: Logger) => { } logger.debug('Reading workflow(s) from checked-out workspace project'); - const workspace = new Workspace(options.workspace); + const workspace = new Workspace(options.workspace, undefined, true, { + branch: options.branch, + }); if (!workspace.valid) { logger.error('Command was run in an invalid openfn workspace'); return; diff --git a/packages/cli/test/projects/checkout.test.ts b/packages/cli/test/projects/checkout.test.ts index 2d838f22d..fb6d69e7a 100644 --- a/packages/cli/test/projects/checkout.test.ts +++ b/packages/cli/test/projects/checkout.test.ts @@ -222,6 +222,15 @@ test.serial('checkout: same id as active', async (t) => { ); }); +test.serial('checkout: makes git ignore the checkout state', async (t) => { + await checkoutHandler( + { command: 'project-checkout', project: 'my-project', workspace: '/ws' }, + logger + ); + + t.is(fs.readFileSync('/ws/.openfn/.gitignore', 'utf8'), '*\n'); +}); + test.serial( 'checkout: writes forked_from based on version history', async (t) => { @@ -233,8 +242,10 @@ test.serial( logger ); - const openfn = yamlToJson(fs.readFileSync('/ws/openfn.yaml', 'utf8')); - t.deepEqual(openfn.project.forked_from, { + const checkout = yamlToJson( + fs.readFileSync('/ws/.openfn/checkout.yaml', 'utf8') + ); + t.deepEqual(checkout.forked_from, { 'simple-workflow-main': 'a', 'another-workflow-main': 'b', }); @@ -451,19 +462,24 @@ test.serial('respect openfn.yaml settings', async (t) => { const yaml = fs.readFileSync('/ws1/openfn.yaml', 'utf8'); t.is( yaml, - `project: - uuid: - id: staging - name: Staging -workspace: - credentials: credentials.yaml - dirs: - projects: p - workflows: w - formats: - openfn: yaml - project: json - workflow: json + `credentials: credentials.yaml +dirs: + projects: p + workflows: w +formats: + openfn: yaml + project: json + workflow: json +` + ); + + // project metadata is written to the checkout file + const checkout = fs.readFileSync('/ws1/.openfn/checkout.yaml', 'utf8'); + t.is( + checkout, + `id: staging +name: Staging +uuid: ` ); @@ -729,7 +745,7 @@ test.serial( ); test.serial( - 'checkout: writes fetched collections into openfn.yaml', + 'checkout: writes fetched collections into resources.yaml', async (t) => { mock({ '/ws6/workflows': {}, @@ -754,12 +770,18 @@ test.serial( logger ); + const resources: any = yamlToJson( + fs.readFileSync('/ws6/resources.yaml', 'utf8') + ); + // ids should never be written to disk - only names + t.deepEqual(resources.collections, { + 'my-collection': {}, + 'another-collection': {}, + }); + + // collections don't belong in openfn.yaml const openfn: any = yamlToJson(fs.readFileSync('/ws6/openfn.yaml', 'utf8')); - // ids should never be written to disk - only bare names - t.deepEqual(openfn.project.collections, [ - 'my-collection', - 'another-collection', - ]); + t.falsy(openfn.collections); } ); @@ -913,8 +935,10 @@ test.serial( ); // assert that staging was checked out ok - let openfn = yamlToJson(fs.readFileSync('/tmp/openfn.yaml', 'utf8')); - t.is(openfn.project.id, 'foo'); + let openfn = yamlToJson( + fs.readFileSync('/tmp/.openfn/checkout.yaml', 'utf8') + ); + t.is(openfn.id, 'foo'); let expression = fs.readFileSync('/tmp/workflows/a/aaa.js', 'utf8'); t.is(expression, '// abc'); @@ -931,8 +955,8 @@ test.serial( logger._reset(); // assert that main was checked out ok - openfn = yamlToJson(fs.readFileSync('/tmp/openfn.yaml', 'utf8')); - t.is(openfn.project.id, 'bar'); + openfn = yamlToJson(fs.readFileSync('/tmp/.openfn/checkout.yaml', 'utf8')); + t.is(openfn.id, 'bar'); expression = fs.readFileSync('/tmp/workflows/a/aaa.js', 'utf8'); t.is(expression, '// 2'); @@ -960,8 +984,10 @@ test.serial( logger._reset(); // assert that main was checked out ok - let openfn = yamlToJson(fs.readFileSync('/tmp/openfn.yaml', 'utf8')); - t.is(openfn.project.id, 'bar'); + let openfn = yamlToJson( + fs.readFileSync('/tmp/.openfn/checkout.yaml', 'utf8') + ); + t.is(openfn.id, 'bar'); let expression = fs.readFileSync('/tmp/workflows/a/aaa.js', 'utf8'); t.is(expression, '// 2'); @@ -977,8 +1003,8 @@ test.serial( ); // assert that staging was checked out ok - openfn = yamlToJson(fs.readFileSync('/tmp/openfn.yaml', 'utf8')); - t.is(openfn.project.id, 'foo'); + openfn = yamlToJson(fs.readFileSync('/tmp/.openfn/checkout.yaml', 'utf8')); + t.is(openfn.id, 'foo'); expression = fs.readFileSync('/tmp/workflows/a/aaa.js', 'utf8'); t.is(expression, '// abc'); @@ -1005,8 +1031,10 @@ test.serial( logger._reset(); // assert that main was checked out ok - let openfn = yamlToJson(fs.readFileSync('/tmp/openfn.yaml', 'utf8')); - t.is(openfn.project.id, 'bar'); + let openfn = yamlToJson( + fs.readFileSync('/tmp/.openfn/checkout.yaml', 'utf8') + ); + t.is(openfn.id, 'bar'); // Now make a change - on checkout, this change will be lost (it is not saved anywhere) fs.writeFileSync('/tmp/workflows/a/aaa.js', 'foobar'); diff --git a/packages/cli/test/projects/clean.test.ts b/packages/cli/test/projects/clean.test.ts index f953c3244..1d0c646cb 100644 --- a/packages/cli/test/projects/clean.test.ts +++ b/packages/cli/test/projects/clean.test.ts @@ -115,3 +115,43 @@ test.serial( ); } ); + +test.serial( + 'does not delete anything if it is not known which project is checked out', + async (t) => { + // two tracked projects, but no checkout metadata anywhere + mock({ + '/ws/workflows/old-workflow': { 'old-job.js': 'fn(s => s)' }, + '/ws/openfn.yaml': '', + '/ws/.projects/main@app.openfn.org.yaml': projectStateFile, + '/ws/.projects/staging@app.openfn.org.yaml': projectStateFile, + }); + const exitCode = process.exitCode; + + await t.throwsAsync( + cleanHandler({ workspace: '/ws', force: true } as any, logger), + { message: 'No checked out project found' } + ); + process.exitCode = exitCode; + + t.true(fs.existsSync('/ws/workflows/old-workflow/old-job.js')); + } +); + +test.serial( + 'does not delete anything if there is no active project to restore', + async (t) => { + // a workspace with no projects and no checkout metadata at all + mock({ + '/ws/workflows/old-workflow': { 'old-job.js': 'fn(s => s)' }, + '/ws/openfn.yaml': '', + }); + + await t.throwsAsync( + cleanHandler({ workspace: '/ws', force: true } as any, logger), + { message: /No active project found/ } + ); + + t.true(fs.existsSync('/ws/workflows/old-workflow/old-job.js')); + } +); diff --git a/packages/cli/test/projects/deploy.test.ts b/packages/cli/test/projects/deploy.test.ts index d27f65701..f0f545e10 100644 --- a/packages/cli/test/projects/deploy.test.ts +++ b/packages/cli/test/projects/deploy.test.ts @@ -30,6 +30,7 @@ import { myProject_v1_spec, } from './fixtures'; import { checkout } from '../../src/projects'; +import { rimraf } from 'rimraf'; let server: any; let strictServer: any; @@ -120,11 +121,17 @@ test.serial( } ); +// project metadata lives in the checkout file, not openfn.yaml +const readCheckout = () => + fs.existsSync('/ws/.openfn/checkout.yaml') + ? fs.readFileSync('/ws/.openfn/checkout.yaml', 'utf8') + : undefined; + test.serial( 'deploy with checkout: true updates the local workspace', async (t) => { await setup(); - const before = fs.readFileSync('/ws/openfn.yaml', 'utf8'); + const before = readCheckout(); await deploy( { @@ -137,7 +144,7 @@ test.serial( logger ); - t.not(fs.readFileSync('/ws/openfn.yaml', 'utf8'), before); + t.not(readCheckout(), before); } ); @@ -146,6 +153,7 @@ test.serial( async (t) => { await setup(); const before = fs.readFileSync('/ws/openfn.yaml', 'utf8'); + const checkoutBefore = readCheckout(); const filesBefore = fs.readdirSync('/ws/.projects'); await deploy( @@ -161,6 +169,7 @@ test.serial( t.is(Object.keys(server.state.projects).length, 2); t.is(fs.readFileSync('/ws/openfn.yaml', 'utf8'), before); + t.is(readCheckout(), checkoutBefore); t.deepEqual(fs.readdirSync('/ws/.projects'), filesBefore); t.truthy(logger._find('success', /Created new project at/)); } @@ -611,11 +620,12 @@ test.serial( await setup(projectYaml); - // user hand-edits openfn.yaml: keep one, drop one, add a new one - + // user hand-edits resources.yaml: keep one, drop one, add a new one - // no workflow files are touched - const openfn: any = yamlToJson(fs.readFileSync('/ws/openfn.yaml', 'utf8')); - openfn.project.collections = ['keep-me', 'new-collection']; - await writeFile('/ws/openfn.yaml', jsonToYaml(openfn)); + await writeFile( + '/ws/resources.yaml', + jsonToYaml({ collections: { 'keep-me': {}, 'new-collection': {} } }) + ); await deploy( { @@ -627,6 +637,11 @@ test.serial( logger ); + // the diff should say what's being deleted and added + t.truthy(logger._find('always', /Collections:/)); + t.truthy(logger._find('always', /remove-me: removed/)); + t.truthy(logger._find('always', /new-collection: added/)); + // a collections-only edit must not be treated as "nothing to deploy" t.falsy(logger._find('success', /Nothing to deploy/)); t.truthy(logger._find('success', /Updated project at/)); @@ -641,6 +656,12 @@ test.serial( ); t.truthy(created?.id); t.falsy(created?.delete); + + // resources.yaml is kept in sync after the deploy + const resources: any = yamlToJson( + fs.readFileSync('/ws/resources.yaml', 'utf8') + ); + t.deepEqual(resources.collections, { 'keep-me': {}, 'new-collection': {} }); } ); @@ -1135,6 +1156,34 @@ test('collectionsChanged: true when a name was removed locally', (t) => { t.true(collectionsChanged(local, remote)); }); +test('collectionsChanged: false when collections are not managed locally', (t) => { + // undefined means "leave alone", unlike an empty list + const local = {} as unknown as Project; + const remote = { + collections: [{ uuid: 'uuid-a', name: 'a' }], + } as unknown as Project; + + t.false(collectionsChanged(local, remote)); +}); + +test('collectionsChanged: true when every collection was removed locally', (t) => { + const local = { collections: [] } as unknown as Project; + const remote = { + collections: [{ uuid: 'uuid-a', name: 'a' }], + } as unknown as Project; + + t.true(collectionsChanged(local, remote)); +}); + +test('deletedCollections: nothing to delete when collections are not managed locally', (t) => { + const merged = {} as unknown as Project; + const remote = { + collections: [{ uuid: 'uuid-a', name: 'a' }], + } as unknown as Project; + + t.deepEqual(deletedCollections(merged, remote), []); +}); + test('deletedCollections: flags a remote name missing from the merged project', (t) => { const merged = { collections: [{ name: 'keep-me' }], @@ -1240,6 +1289,62 @@ test('printRichDiff: should report removed channels', (t) => { t.truthy(logger._find('always', /one: removed/)); }); +test('printRichDiff: should report added and removed collections', (t) => { + const local = new Project({ + name: 'local', + workflows: [], + collections: [{ name: 'keep-me' }, { name: 'new-one' }], + }); + const remote = new Project({ + name: 'remote', + workflows: [], + collections: [ + { uuid: 'a', name: 'keep-me' }, + { uuid: 'b', name: 'old-one' }, + ], + }); + + printRichDiff(local, remote, [], logger); + + t.truthy(logger._find('always', /following changes to the remote project/)); + t.truthy(logger._find('always', /Collections:/)); + t.truthy(logger._find('always', /new-one: added/)); + t.truthy(logger._find('always', /old-one: removed/)); +}); + +test('printRichDiff: should show every collection as removed when all are deleted', (t) => { + const local = new Project({ name: 'local', workflows: [], collections: [] }); + const remote = new Project({ + name: 'remote', + workflows: [], + collections: [ + { uuid: 'a', name: 'one' }, + { uuid: 'b', name: 'two' }, + ], + }); + + printRichDiff(local, remote, [], logger); + + t.truthy(logger._find('always', /one: removed/)); + t.truthy(logger._find('always', /two: removed/)); +}); + +test('printRichDiff: should not report collections that are not managed locally', (t) => { + const local = new Project({ name: 'local', workflows: [] }); + const remote = new Project({ + name: 'remote', + workflows: [], + collections: [{ uuid: 'a', name: 'one' }], + }); + + // use a private logger: an absence check isn't safe on the shared one + const privateLogger = createMockLogger(undefined, { level: 'debug' }); + printRichDiff(local, remote, [], privateLogger); + + t.falsy(privateLogger._find('always', /Collections:/)); + t.truthy(privateLogger._find('info', /No workflow changes detected/)); +}); + test('printRichDiff: should list changed channel fields', (t) => { const local = new Project({ name: 'local', @@ -1257,3 +1362,140 @@ test('printRichDiff: should list changed channel fields', (t) => { t.truthy(logger._find('always', /one: changed/)); t.truthy(logger._find('always', /enabled: "true" -> "false"/)); }); + +const twoProjectsNoCheckout = () => + mockFs({ + '/ws/.projects/main@localhost.yaml': projectYaml, + '/ws/.projects/staging@localhost.yaml': projectYaml, + '/ws/openfn.yaml': '', + }); + +test.serial( + 'deploy: aborts if it is not known which project is checked out', + async (t) => { + twoProjectsNoCheckout(); + const exitCode = process.exitCode; + + await t.throwsAsync( + deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + confirm: false, + } as any, + logger + ), + { message: 'No checked out project found' } + ); + process.exitCode = exitCode; + } +); + +test.serial( + 'deploy: does not look for the checked out project with checkout: false', + async (t) => { + twoProjectsNoCheckout(); + // use a private logger: an absence check isn't safe on the shared one + const privateLogger = createMockLogger(undefined, { level: 'debug' }); + + // this deploy doesn't go anywhere, but it must not stop to ask + await deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + confirm: false, + checkout: false, + } as any, + privateLogger + ).catch(() => {}); + + t.falsy(privateLogger._find('error', /No checked out project found/)); + } +); + +test.serial( + 'deploy: does not look for the checked out project if there is an explicit target', + async (t) => { + twoProjectsNoCheckout(); + const privateLogger = createMockLogger(undefined, { level: 'debug' }); + + // this deploy doesn't go anywhere, but it must not stop to ask + await deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + confirm: false, + project: 'staging', + } as any, + privateLogger + ).catch(() => {}); + + t.falsy(privateLogger._find('error', /No checked out project found/)); + } +); + +test.serial( + 'deploy: merges the workspace into an explicit target on a fresh clone, without a checkout', + async (t) => { + await setup(projectYaml); + + // simulate a fresh git clone: the workflows are there, nothing is checked + // out, and there's more than one tracked project + // (fs.rmSync is a no-op under mock-fs, hence rimraf) + await rimraf('/ws/.openfn'); + t.false(fs.existsSync('/ws/.openfn')); + fs.writeFileSync('/ws/.projects/other@localhost.yaml', projectYaml); + await writeFile('/ws/workflows/my-workflow/transform-data.js', 'log()'); + + await deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: 'main', + checkout: false, + force: true, + confirm: false, + } as any, + logger + ); + + t.truthy(logger._find('success', /Updated project at/)); + t.is( + server.state.projects[UUID].workflows['my-workflow'].jobs[ + 'transform-data' + ].body, + 'log()' + ); + // deploying doesn't set up a checkout + t.false(fs.existsSync('/ws/.openfn')); + } +); + +test.serial( + 'deploy: deploys a project file on a fresh clone, without a checkout', + async (t) => { + mockFs({ + '/ws/project.yaml': myProject_spec, + '/ws/openfn.yaml': '', + '/ws/.projects/main@localhost.yaml': projectYaml, + '/ws/.projects/other@localhost.yaml': projectYaml, + }); + + await deploy( + { + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + project: '/ws/project.yaml', + } as any, + logger + ); + + t.truthy(logger._find('success', /Created new project at/)); + t.is(Object.keys(server.state.projects).length, 2); + } +); diff --git a/packages/cli/test/projects/ensure-checkout.test.ts b/packages/cli/test/projects/ensure-checkout.test.ts new file mode 100644 index 000000000..e9ead3df6 --- /dev/null +++ b/packages/cli/test/projects/ensure-checkout.test.ts @@ -0,0 +1,300 @@ +import test from 'ava'; +import fs from 'node:fs'; +import mock from 'mock-fs'; +import { createMockLogger } from '@openfn/logger'; +import Project, { Workspace, yamlToJson } from '@openfn/project'; + +import ensureCheckout, { + describeProject, + getDefaultProject, +} from '../../src/projects/ensure-checkout'; + +const logger = createMockLogger(undefined, { level: 'debug' }); + +test.afterEach(() => { + mock.restore(); + logger._reset(); +}); + +const projectFile = (id: string) => + new Project({ + id, + name: id, + openfn: { uuid: `uuid-${id}`, endpoint: 'https://app.openfn.org' }, + workflows: [{ id: 'wf', name: 'wf', history: ['abc'], steps: [] }], + }).serialize('project', { format: 'yaml' }) as string; + +const setup = (ids = ['main', 'staging'], extra = {}) => + mock({ + '/ws/openfn.yaml': '', + '/ws/workflows/wf/wf.yaml': 'id: wf', + ...Object.fromEntries( + ids.map((id) => [ + `/ws/.projects/${id}@app.openfn.org.yaml`, + projectFile(id), + ]) + ), + ...extra, + }); + +const readCheckout = (path = '/ws/.openfn/checkout.yaml') => + yamlToJson(fs.readFileSync(path, 'utf8')); + +// A prompt that records what it was asked and picks a project by alias +const pickPrompt = (alias: string) => { + const calls: any[] = []; + const prompt = async (projects: Project[], defaultProject: Project) => { + calls.push({ projects, defaultProject }); + return projects.find((p) => p.alias === alias)!; + }; + return { prompt, calls }; +}; + +const neverPrompt = async () => { + throw new Error('should not have prompted'); +}; + +test.serial('does nothing if there is already checkout metadata', async (t) => { + setup(['main', 'staging'], { '/ws/.openfn/checkout.yaml': 'id: main' }); + const workspace = new Workspace('/ws', logger); + + const result = await ensureCheckout(workspace, logger, { + interactive: true, + prompt: neverPrompt, + }); + + t.is(result, workspace); + t.is(readCheckout().id, 'main'); +}); + +test.serial('does nothing if there are no tracked projects', async (t) => { + setup([]); + const workspace = new Workspace('/ws', logger, false); + + const result = await ensureCheckout(workspace, logger, { + interactive: true, + prompt: neverPrompt, + }); + + t.is(result, workspace); + t.false(fs.existsSync('/ws/.openfn')); +}); + +test.serial('uses the only tracked project without prompting', async (t) => { + setup(['main']); + + const result = await ensureCheckout(new Workspace('/ws', logger), logger, { + interactive: true, + prompt: neverPrompt, + }); + + t.is(readCheckout().uuid, 'uuid-main'); + t.is(result.activeProject?.uuid, 'uuid-main'); +}); + +test.serial('prompts if there are several tracked projects', async (t) => { + setup(); + const { prompt, calls } = pickPrompt('staging'); + + const result = await ensureCheckout(new Workspace('/ws', logger), logger, { + interactive: true, + prompt, + }); + + // explain why we're asking + t.truthy(logger._find('info', /Failed to find a checkout.yaml file/)); + t.truthy(logger._find('info', /just pick which project is checked out/)); + + t.is(calls.length, 1); + t.deepEqual(calls[0].projects.map((p: Project) => p.alias).sort(), [ + 'main', + 'staging', + ]); + + // the checkout file has been set up, and the workspace knows about it + t.is(readCheckout().uuid, 'uuid-staging'); + t.is(result.activeProject?.uuid, 'uuid-staging'); +}); + +test.serial( + 'does not set anything up if the user skips the prompt', + async (t) => { + setup(); + const workspace = new Workspace('/ws', logger); + + const result = await ensureCheckout(workspace, logger, { + interactive: true, + prompt: async () => null, + }); + + t.is(result, workspace); + t.false(fs.existsSync('/ws/.openfn')); + t.truthy(logger._find('info', /Skipped/)); + } +); + +test.serial( + 'does not touch the workflows when setting up the checkout', + async (t) => { + setup(); + const { prompt } = pickPrompt('main'); + + await ensureCheckout(new Workspace('/ws', logger), logger, { + interactive: true, + prompt, + }); + + t.deepEqual(fs.readdirSync('/ws').sort(), [ + '.openfn', + '.projects', + 'openfn.yaml', + 'workflows', + ]); + t.is(fs.readFileSync('/ws/workflows/wf/wf.yaml', 'utf8'), 'id: wf'); + t.is(fs.readFileSync('/ws/openfn.yaml', 'utf8'), ''); + } +); + +test.serial('suggests the project matching the git branch', async (t) => { + setup(['main', 'staging']); + const { prompt, calls } = pickPrompt('main'); + + await ensureCheckout( + new Workspace('/ws', logger, true, { branch: 'staging' }), + logger, + { interactive: true, prompt } + ); + + t.is(calls[0].defaultProject.alias, 'staging'); +}); + +test.serial( + 'suggests main if there is no git branch, even if it is not first', + async (t) => { + // projects are listed alphabetically, so main is not first here + setup(['alpha', 'main']); + const { prompt, calls } = pickPrompt('alpha'); + + await ensureCheckout( + new Workspace('/ws', logger, true, { branch: false }), + logger, + { interactive: true, prompt } + ); + + t.is(calls[0].defaultProject.alias, 'main'); + } +); + +test.serial('writes the checkout file for the branch', async (t) => { + setup(); + const { prompt } = pickPrompt('staging'); + + await ensureCheckout( + new Workspace('/ws', logger, true, { branch: 'dev' }), + logger, + { interactive: true, prompt } + ); + + t.is( + readCheckout('/ws/.openfn/branches/dev/checkout.yaml').uuid, + 'uuid-staging' + ); + t.false(fs.existsSync('/ws/.openfn/checkout.yaml')); +}); + +test.serial( + 'aborts without prompting if several projects and not interactive', + async (t) => { + setup(); + const exitCode = process.exitCode; + + await t.throwsAsync( + ensureCheckout(new Workspace('/ws', logger), logger, { + interactive: false, + prompt: neverPrompt, + }), + { message: 'No checked out project found' } + ); + + // abort() flags the process as failed: don't let that leak out of the test + process.exitCode = exitCode; + + t.false(fs.existsSync('/ws/.openfn')); + t.truthy(logger._find('error', /No checked out project found/)); + } +); + +test.serial( + 'carries on without a checkout if not interactive and not required', + async (t) => { + setup(); + const workspace = new Workspace('/ws', logger); + + const result = await ensureCheckout(workspace, logger, { + interactive: false, + required: false, + prompt: neverPrompt, + }); + + t.is(result, workspace); + t.false(fs.existsSync('/ws/.openfn')); + } +); + +test.serial('still prompts if interactive, even if not required', async (t) => { + setup(); + const { prompt, calls } = pickPrompt('staging'); + + const result = await ensureCheckout(new Workspace('/ws', logger), logger, { + interactive: true, + required: false, + prompt, + }); + + t.is(calls.length, 1); + t.is(result.activeProject?.uuid, 'uuid-staging'); +}); + +const projects = (aliases: string[]) => + aliases.map((alias) => ({ alias })) as unknown as Project[]; + +test('getDefaultProject: prefers the alias matching the branch', (t) => { + const result = getDefaultProject(projects(['main', 'staging']), 'staging'); + t.is(result.alias, 'staging'); +}); + +test('getDefaultProject: defaults to main if there is no branch', (t) => { + t.is(getDefaultProject(projects(['a', 'main', 'b'])).alias, 'main'); +}); + +test('getDefaultProject: falls back to the first project', (t) => { + t.is(getDefaultProject(projects(['a', 'b']), 'feature').alias, 'a'); +}); + +const described = (props: object) => describeProject(props as Project); + +test('describeProject: alias with id and uuid', (t) => { + t.is( + described({ alias: 'staging', id: 'my-project', openfn: { uuid: 'abcd' } }), + 'staging (my-project | abcd)' + ); +}); + +test('describeProject: copes with a missing uuid or id', (t) => { + t.is( + described({ alias: 'staging', id: 'my-project' }), + 'staging (my-project)' + ); + t.is( + described({ alias: 'staging', openfn: { uuid: 'abcd' } }), + 'staging (abcd)' + ); +}); + +test('describeProject: just the alias if there is no id or uuid', (t) => { + t.is(described({ alias: 'staging' }), 'staging'); +}); + +test('describeProject: copes with a missing alias', (t) => { + t.is(described({ id: 'my-project' }), '(no alias) (my-project)'); +}); diff --git a/packages/cli/test/projects/list.test.ts b/packages/cli/test/projects/list.test.ts index 0891e5038..aab504a98 100644 --- a/packages/cli/test/projects/list.test.ts +++ b/packages/cli/test/projects/list.test.ts @@ -4,8 +4,14 @@ import { createMockLogger } from '@openfn/logger'; import mock from 'mock-fs'; import { jsonToYaml } from '@openfn/project'; +// a workspace with several projects but no checkout metadata +const noCheckoutProject = jsonToYaml({ id: 'my-project', workflows: [] }); + mock({ 'no-ws/': { 'some.yaml': 'name: smth' }, + '/no-checkout/openfn.yaml': '', + '/no-checkout/.projects/main@app.openfn.org.yaml': noCheckoutProject, + '/no-checkout/.projects/staging@app.openfn.org.yaml': noCheckoutProject, '/ws/openfn.yaml': jsonToYaml({ project: { id: 'my-project', @@ -172,3 +178,23 @@ staging | my-project message as string ); }); + +test.serial( + 'lists projects without a checkout, without prompting or aborting', + async (t) => { + const privateLogger = createMockLogger('', { level: 'debug' }); + + await list( + { command: 'projects', workspace: '/no-checkout' }, + privateLogger + ); + + const { message } = privateLogger._find( + 'always', + /available openfn projects/i + ); + t.regex(message as string, /^main \|/m); + t.regex(message as string, /^staging \|/m); + t.notRegex(message as string, /\(active\)/); + } +); diff --git a/packages/cli/test/projects/util.test.ts b/packages/cli/test/projects/util.test.ts index b4486ee17..3f5b36ce0 100644 --- a/packages/cli/test/projects/util.test.ts +++ b/packages/cli/test/projects/util.test.ts @@ -1,10 +1,18 @@ import test from 'ava'; -import Project, { generateWorkflow } from '@openfn/project'; +import fs from 'node:fs'; +import mock from 'mock-fs'; +import Project, { generateWorkflow, yamlToJson } from '@openfn/project'; import { findLocallyChangedWorkflows, tidyWorkflowDir, + writeCheckoutFile, + ensureCheckoutIgnored, } from '../../src/projects/util'; +test.afterEach(() => { + mock.restore(); +}); + test('tidyWorkflowDir: removes workflows that no longer exist', async (t) => { const currentProject = new Project({ name: 'current', @@ -271,3 +279,114 @@ test('findLocallyChangedWorkflows: detect 1 locally removed workflow', async (t) const changed = await findLocallyChangedWorkflows(workspace, project); t.deepEqual(changed, ['b']); }); + +const trackedProject = () => + new Project({ + id: 'my-project', + name: 'My Project', + openfn: { uuid: 'abcd', endpoint: 'https://app.openfn.org' }, + workflows: [ + { id: 'wf', name: 'wf', history: ['old', 'latest'], steps: [] }, + ], + }); + +test.serial( + 'writeCheckoutFile: writes the project meta and forked_from to the checkout file', + async (t) => { + mock({ '/ws': {} }); + + const filePath = await writeCheckoutFile('/ws', trackedProject()); + + t.is(filePath, '/ws/.openfn/checkout.yaml'); + t.deepEqual(yamlToJson(fs.readFileSync(filePath, 'utf8')), { + endpoint: 'https://app.openfn.org', + forked_from: { wf: 'latest' }, + id: 'my-project', + name: 'My Project', + uuid: 'abcd', + }); + } +); + +test.serial( + 'writeCheckoutFile: writes to the checkout file for a branch', + async (t) => { + mock({ '/ws': {} }); + + const filePath = await writeCheckoutFile('/ws', trackedProject(), 'dev'); + + t.is(filePath, '/ws/.openfn/branches/dev/checkout.yaml'); + t.true(fs.existsSync(filePath)); + t.false(fs.existsSync('/ws/.openfn/checkout.yaml')); + } +); + +test.serial( + 'writeCheckoutFile: does not touch the workspace config or workflows', + async (t) => { + mock({ + '/ws/openfn.yaml': 'credentials: creds.yaml', + '/ws/workflows/wf/wf.yaml': 'id: wf', + }); + + await writeCheckoutFile('/ws', trackedProject()); + + t.deepEqual(fs.readdirSync('/ws').sort(), [ + '.openfn', + 'openfn.yaml', + 'workflows', + ]); + t.is(fs.readFileSync('/ws/openfn.yaml', 'utf8'), 'credentials: creds.yaml'); + t.is(fs.readFileSync('/ws/workflows/wf/wf.yaml', 'utf8'), 'id: wf'); + t.deepEqual(fs.readdirSync('/ws/workflows/wf'), ['wf.yaml']); + } +); + +test.serial( + 'ensureCheckoutIgnored: makes git ignore everything in .openfn', + async (t) => { + mock({ '/ws': {} }); + + await ensureCheckoutIgnored('/ws'); + + t.is(fs.readFileSync('/ws/.openfn/.gitignore', 'utf8'), '*\n'); + } +); + +test.serial( + 'ensureCheckoutIgnored: leaves an existing .gitignore alone', + async (t) => { + mock({ '/ws/.openfn/.gitignore': 'custom\n' }); + + await ensureCheckoutIgnored('/ws'); + + t.is(fs.readFileSync('/ws/.openfn/.gitignore', 'utf8'), 'custom\n'); + } +); + +test.serial( + 'ensureCheckoutIgnored: does nothing if OPENFN_IGNORE_CHECKOUT_META is set', + async (t) => { + mock({ '/ws': {} }); + process.env.OPENFN_IGNORE_CHECKOUT_META = 'true'; + + try { + await ensureCheckoutIgnored('/ws'); + } finally { + delete process.env.OPENFN_IGNORE_CHECKOUT_META; + } + + t.false(fs.existsSync('/ws/.openfn')); + } +); + +test.serial( + 'writeCheckoutFile: makes git ignore the checkout state', + async (t) => { + mock({ '/ws': {} }); + + await writeCheckoutFile('/ws', trackedProject()); + + t.is(fs.readFileSync('/ws/.openfn/.gitignore', 'utf8'), '*\n'); + } +); diff --git a/packages/lexicon/core.d.ts b/packages/lexicon/core.d.ts index 399b27e50..59f38b85f 100644 --- a/packages/lexicon/core.d.ts +++ b/packages/lexicon/core.d.ts @@ -175,7 +175,8 @@ export interface WorkspaceFileLegacy { // Structure of the new openfn.yaml file export interface WorkspaceFile { workspace: WorkspaceConfig; - project: ProjectMeta; + // legacy: project meta now lives in .openfn/checkout.yaml + project?: ProjectMeta; } export interface WorkspaceConfig { diff --git a/packages/project/src/Project.ts b/packages/project/src/Project.ts index fee67463a..b81136b31 100644 --- a/packages/project/src/Project.ts +++ b/packages/project/src/Project.ts @@ -12,7 +12,8 @@ import { getUuidForEdge, getUuidForStep } from './util/uuid'; import { merge, MergeProjectOptions } from './merge/merge-project'; import { diff as projectDiff } from './util/project-diff'; import { Workspace } from './Workspace'; -import { buildConfig, extractConfig } from './util/config'; +import { buildConfig, extractCheckout, extractConfig } from './util/config'; +import { RESOURCES_FILE, updateResourceCollections } from './util/resources'; import { Provisioner } from '@openfn/lexicon/lightning'; import { SandboxMeta, @@ -311,6 +312,24 @@ export class Project { return extractConfig(this); } + /** + * Generates the contents of the checkout file, plus its file path + */ + generateCheckout(branch?: string | false | null) { + return extractCheckout(this, branch); + } + + /** + * Generates the contents of resources.yaml with this project's collections + * merged into an existing file (other keys are left alone) + */ + generateResources(existing?: string) { + return { + path: RESOURCES_FILE, + content: updateResourceCollections(this, existing), + }; + } + clone() { return new Project(this.serialize('project') as any); } diff --git a/packages/project/src/Workspace.ts b/packages/project/src/Workspace.ts index f7e2bc3ec..018b21a78 100644 --- a/packages/project/src/Workspace.ts +++ b/packages/project/src/Workspace.ts @@ -9,12 +9,19 @@ import { buildConfig, loadWorkspaceFile, findWorkspaceFile, + loadCheckoutFile, } from './util/config'; import fromProject from './parse/from-project'; import type { Logger } from '@openfn/logger'; import matchProject from './util/match-project'; import { extractAliasFromFilename } from './parse/from-path'; +export type WorkspaceOptions = { + // The git branch the workspace is on, which determines which checkout file + // to use. Pass false or null (or leave unset) to use the default checkout + branch?: string | false | null; +}; + export class Workspace { // @ts-ignore config not definitely assigned - it sure is config: l.WorkspaceConfig; @@ -24,6 +31,8 @@ export class Workspace { root: string; + branch?: string | false | null; + private projects: Project[] = []; private projectPaths = new Map(); private isValid: boolean = false; @@ -31,8 +40,14 @@ export class Workspace { // Set validate to false to suppress warnings if a Workspace doesn't exist // This is appropriate if, say, fetching a project for the first time - constructor(workspacePath: string, logger?: Logger, validate = true) { + constructor( + workspacePath: string, + logger?: Logger, + validate = true, + options: WorkspaceOptions = {} + ) { this.root = workspacePath; + this.branch = options.branch; this.logger = logger ?? createLogger('Workspace', { level: 'info' }); let context = { workspace: undefined, project: undefined }; @@ -55,7 +70,11 @@ export class Workspace { // it's not super reliable // Actually would it not be better to find the ACTUAL project and just // reference that? - this.activeProject = context.project; + this.activeProject = loadCheckoutFile( + workspacePath, + this.branch, + context.project + ); const projectsPath = path.join(workspacePath, this.config.dirs.projects); // dealing with projects @@ -134,6 +153,7 @@ export class Workspace { return await Project.from('fs', { root: this.root, config: this.config, + branch: this.branch, // The checked out project can't meaningfully be said to have an alias // But we can force one if it makes sense from context alias: alias ?? null, diff --git a/packages/project/src/index.ts b/packages/project/src/index.ts index 2898043c6..f7a87bf1c 100644 --- a/packages/project/src/index.ts +++ b/packages/project/src/index.ts @@ -12,10 +12,14 @@ export { diff } from './util/project-diff'; export type { WorkflowDiff, ChannelDiff, + CollectionDiff, ProjectDiff, DiffType, } from './util/project-diff'; +export { RESOURCES_FILE } from './util/resources'; +export { hasCheckoutMeta } from './util/config'; + export { generateStepDiff, generateEdgeDiff } from './util/workflow-diff'; export type { StepChange, diff --git a/packages/project/src/merge/merge-project.ts b/packages/project/src/merge/merge-project.ts index 9d1186189..af6b5f770 100644 --- a/packages/project/src/merge/merge-project.ts +++ b/packages/project/src/merge/merge-project.ts @@ -202,11 +202,16 @@ export function merge( return merged; } +// If the source has no collections defined, they aren't managed locally and +// the target's are left alone. An empty list means remove them all export function mergeCollections( - source: CollectionState[] = [], - target: CollectionState[] = [] -): CollectionState[] { - const targetByName = new Map(target.map((c) => [c.name, c])); + source?: CollectionState[], + target?: CollectionState[] +): CollectionState[] | undefined { + if (!source) { + return target; + } + const targetByName = new Map((target ?? []).map((c) => [c.name, c])); return source.map(({ name }) => ({ name, uuid: targetByName.get(name)?.uuid, diff --git a/packages/project/src/parse/from-fs.ts b/packages/project/src/parse/from-fs.ts index 446a4838a..e104702f4 100644 --- a/packages/project/src/parse/from-fs.ts +++ b/packages/project/src/parse/from-fs.ts @@ -9,12 +9,17 @@ import { buildConfig, loadWorkspaceFile, findWorkspaceFile, + loadCheckoutFile, } from '../util/config'; import { omit } from 'lodash-es'; import { Logger } from '@openfn/logger'; import omitNil from '../util/omit-nil'; import slugify from '../util/slugify'; -import { RESOURCES_FILE, fromResourceChannels } from '../util/resources'; +import { + RESOURCES_FILE, + fromResourceChannels, + fromResourceCollections, +} from '../util/resources'; export type FromFsConfig = { root: string; @@ -22,6 +27,8 @@ export type FromFsConfig = { logger?: Logger; alias?: string | null; name?: string; + // git branch, used to locate the checkout file + branch?: string | false | null; }; // Parse a single project from a root folder @@ -34,33 +41,41 @@ export const parseProject = async (options: FromFsConfig) => { const { type, content } = findWorkspaceFile(root); const context = loadWorkspaceFile(content, type as any); const config = buildConfig(options.config ?? context.workspace); + const checkout = + loadCheckoutFile(root, options.branch, context.project) ?? {}; const proj: any = { - id: options.name ? slugify(options.name) : context.project?.id, - name: options.name ? slugify(options.name) : context.project?.name, - openfn: omit(context.project, ['id', 'forked_from', 'collections']), - // openfn.yaml only ever holds bare collection names - no uuids, those - // belong to the server - collections: context.project.collections?.map((name: string) => ({ + id: options.name ? slugify(options.name) : checkout.id, + name: options.name ? slugify(options.name) : checkout.name, + openfn: omit(checkout, ['id', 'forked_from', 'collections']), + + // Legacy: collections used to be a list of names in openfn.yaml. A + // collections key in resources.yaml takes precedence (see below) + collections: context.collections?.map((name: string) => ({ name, })), config: config, workflows: [], cli: omitNil({ - forked_from: context.project.forked_from, + forked_from: checkout.forked_from, }), }; - // resources.yaml is optional: if it's missing, or has no channels key, - // channels stay undefined and are left untouched on merge/deploy + // resources.yaml is optional: if it's missing, or has no channels or + // collections key, they stay undefined and are left untouched on + // merge/deploy const resources = await fs .readFile(path.resolve(root, RESOURCES_FILE), 'utf-8') .catch((e) => { if (e.code !== 'ENOENT') throw e; }); - const channels = resources && yamlToJson(resources)?.channels; - if (channels) { - proj.channels = fromResourceChannels(channels); + const parsed = resources ? yamlToJson(resources) : undefined; + if (parsed?.channels) { + proj.channels = fromResourceChannels(parsed.channels); + } + if (parsed && 'collections' in parsed) { + // an empty key means no collections (not the same as no key!) + proj.collections = fromResourceCollections(parsed.collections); } // now find all the workflows diff --git a/packages/project/src/serialize/to-fs.ts b/packages/project/src/serialize/to-fs.ts index 097a586c0..6293ce131 100644 --- a/packages/project/src/serialize/to-fs.ts +++ b/packages/project/src/serialize/to-fs.ts @@ -3,28 +3,32 @@ import { omit } from 'lodash-es'; import { Project } from '../Project'; import { jsonToYaml } from '../util/yaml'; -import { extractConfig } from '../util/config'; -import { RESOURCES_FILE, toResourceChannels } from '../util/resources'; +import { extractCheckout, extractConfig } from '../util/config'; +import { RESOURCES_FILE, toResources } from '../util/resources'; const stringify = (json: any) => JSON.stringify(json, null, 2); type ToFsOptions = { // private: stamp schema_version on each workflow file. off by default includeSchemaVersion?: boolean; + // git branch, used to locate the checkout file + branch?: string | false | null; }; export default function (project: Project, options: ToFsOptions = {}) { const files: Record = {}; - const { path, content } = extractConfig(project); - files[path] = content; + const config = extractConfig(project); + files[config.path] = config.content; - // Only write resources.yaml if the project knows about channels. A missing - // file means channels are not managed locally - if (project.channels) { - files[RESOURCES_FILE] = jsonToYaml({ - channels: toResourceChannels(project.channels, project.credentials), - }); + const checkout = extractCheckout(project, options.branch); + files[checkout.path] = checkout.content; + + // Only write resources.yaml if the project knows about channels or + // collections. A missing key means they are not managed locally + const resources = toResources(project); + if (resources) { + files[RESOURCES_FILE] = resources; } for (const wf of project.workflows) { diff --git a/packages/project/src/util/config.ts b/packages/project/src/util/config.ts index bef3e625d..e4fe7dbe7 100644 --- a/packages/project/src/util/config.ts +++ b/packages/project/src/util/config.ts @@ -5,6 +5,22 @@ import { pickBy, isNil } from 'lodash-es'; import { yamlToJson, jsonToYaml } from './yaml'; import Project from '../Project'; +// Recursively sort object keys so that config files serialize stably +const sortKeys = (value: any): any => { + if (Array.isArray(value)) { + return value.map(sortKeys); + } + if (value && typeof value === 'object') { + return Object.keys(value) + .sort() + .reduce((obj: any, key) => { + obj[key] = sortKeys(value[key]); + return obj; + }, {}); + } + return value; +}; + // Initialize and default Workspace (and Project) config export const buildConfig = (config: Partial = {}) => ({ @@ -22,8 +38,23 @@ export const buildConfig = (config: Partial = {}) => ({ }, }); -// Generate a workspace config (openfn.yaml) file for a project -export const extractConfig = (source: Project, format?: 'yaml' | 'json') => { +// The checkout file tracks which project is expanded into the workflows dir, +// plus any transient sync state (like forked_from, which helps track divergence). +// +// It's local to each machine and git-ignored, so git can't switch it when you +// change branch. To keep the CLI aware of what's checked out after a branch +// switch (without the user having to re-run `openfn checkout`), each git +// branch gets its own checkout file. +export const getCheckoutPath = (branch?: string | false | null) => + branch + ? path.join('.openfn', 'branches', branch, 'checkout.yaml') + : path.join('.openfn', 'checkout.yaml'); + +// Generate the checkout file for a project +export const extractCheckout = ( + source: Project, + branch?: string | false | null +) => { const project: any = { ...(source.openfn || {}), id: source.id, @@ -36,28 +67,49 @@ export const extractConfig = (source: Project, format?: 'yaml' | 'json') => { project.forked_from = source.cli.forked_from; } - if (source.collections?.length) { - // openfn.yaml only ever carries collection names - no ids/uuids, those - // belong to the server. - project.collections = source.collections.map((c) => c.name); + return { + path: getCheckoutPath(branch), + content: jsonToYaml(sortKeys(project)), + }; +}; + +// Load project metadata from the checkout file +// If there's no checkout file, fall back to the legacy project block +// in openfn.yaml (which will be migrated on the next write) +export const loadCheckoutFile = ( + root: string = '.', + branch?: string | false | null, + legacyProject?: l.ProjectMeta +): l.ProjectMeta | undefined => { + try { + const content = readFileSync( + path.resolve(root, getCheckoutPath(branch)), + 'utf8' + ); + return (yamlToJson(content) as l.ProjectMeta) ?? {}; + } catch (e) { + if (legacyProject && Object.keys(legacyProject).length) { + return legacyProject; + } } +}; +// Generate a workspace config (openfn.yaml) file for a project +export const extractConfig = (source: Project, format?: 'yaml' | 'json') => { const workspace = { ...source.config, }; - const content = { project, workspace }; - format = format ?? workspace.formats.openfn; if (format === 'yaml') { return { path: 'openfn.yaml', - content: jsonToYaml(content), + content: jsonToYaml(sortKeys(workspace)), }; } return { path: 'openfn.json', - content: JSON.stringify(content, null, 2), + content: JSON.stringify(sortKeys(workspace), null, 2), }; }; @@ -65,7 +117,7 @@ export const loadWorkspaceFile = ( contents: string | l.WorkspaceFile | l.WorkspaceFileLegacy, format: 'yaml' | 'json' = 'yaml' ) => { - let project, workspace; + let project, workspace, collections: string[] | undefined; let json: any = contents; if (format === 'yaml') { json = yamlToJson(contents as any) ?? {}; @@ -73,6 +125,8 @@ export const loadWorkspaceFile = ( json = JSON.parse(contents); } + // Flat format: top level keys are workspace config (plus a legacy project block) + // Nested format: { workspace, project } const legacy = !json.workspace && !json.projects; if (legacy) { project = json.project ?? {}; @@ -86,9 +140,13 @@ export const loadWorkspaceFile = ( dirs, project: _ /* ignore!*/, name, + collections: flatCollections, ...rest } = json; + // Collections live at the top level, but may be in a legacy project block + collections = flatCollections ?? project.collections; + workspace = pickBy( { ...rest, @@ -100,9 +158,10 @@ export const loadWorkspaceFile = ( } else { project = json.project ?? {}; workspace = json.workspace ?? {}; + collections = project.collections; } - return { project, workspace }; + return { project, workspace, collections }; }; export const findWorkspaceFile = (dir: string = '.') => { @@ -126,3 +185,22 @@ export const findWorkspaceFile = (dir: string = '.') => { } return { content, type }; }; + +// Does this workspace know which project is checked out? +// True if there's checkout metadata in the checkout file, or in a legacy +// project block in openfn.yaml. An empty checkout file doesn't count +export const hasCheckoutMeta = ( + root: string = '.', + branch?: string | false | null +) => { + let legacyProject: l.ProjectMeta | undefined; + try { + const { type, content } = findWorkspaceFile(root); + legacyProject = loadWorkspaceFile(content, type as any).project; + } catch (e) { + // No workspace file: there can't be a legacy project block + } + + const meta = loadCheckoutFile(root, branch, legacyProject); + return !!meta && Object.keys(meta).length > 0; +}; diff --git a/packages/project/src/util/project-diff.ts b/packages/project/src/util/project-diff.ts index de0d5927b..f942bfad3 100644 --- a/packages/project/src/util/project-diff.ts +++ b/packages/project/src/util/project-diff.ts @@ -17,10 +17,16 @@ export type ChannelDiff = { changes?: Record; }; +export type CollectionDiff = { + name: string; + type: Exclude; +}; + export type ProjectDiff = { workflows: WorkflowDiff[]; resources: { channels: ChannelDiff[]; + collections: CollectionDiff[]; }; }; @@ -34,7 +40,7 @@ export type ProjectDiff = { * @param b - The comparison project (e.g., staging branch) * Channels are compared by their resources.yaml id. If either project has no * channels defined (undefined) they are not managed, so no channel changes are - * reported. + * reported. Collections are compared by name, and follow the same rule. * * @returns An object with `workflows` and `resources` indicating how B differs * from A. Each workflow diff is: @@ -87,9 +93,32 @@ export function diff( } } - return { workflows: diffs, resources: { channels: diffChannels(a, b) } }; + return { + workflows: diffs, + resources: { + channels: diffChannels(a, b), + collections: diffCollections(a, b), + }, + }; } +const diffCollections = (a: Project, b: Project): CollectionDiff[] => { + if (!a.collections || !b.collections) { + return []; + } + + const namesA = new Set(a.collections.map((c) => c.name)); + const namesB = new Set(b.collections.map((c) => c.name)); + return [ + ...Array.from(namesA) + .filter((name) => !namesB.has(name)) + .map((name) => ({ name, type: 'removed' as const })), + ...Array.from(namesB) + .filter((name) => !namesA.has(name)) + .map((name) => ({ name, type: 'added' as const })), + ]; +}; + const diffChannels = (a: Project, b: Project): ChannelDiff[] => { if (!a.channels || !b.channels) { return []; diff --git a/packages/project/src/util/resources.ts b/packages/project/src/util/resources.ts index 0495cea8a..6f21905a3 100644 --- a/packages/project/src/util/resources.ts +++ b/packages/project/src/util/resources.ts @@ -1,6 +1,8 @@ import type l from '@openfn/lexicon'; import getCredentialName from './get-credential-name'; import slugify from './slugify'; +import { jsonToYaml, yamlToJson } from './yaml'; +import type { Project } from '../Project'; // resources.yaml holds server-side resources, keyed by type (channels, ...) export const RESOURCES_FILE = 'resources.yaml'; @@ -65,3 +67,57 @@ export const fromResourceChannels = ( ...(c.credential && { destination_credential_id: c.credential }), }; }); + +// Collections are keyed by name. The values are empty for now, but this +// leaves room for per-collection settings later (no uuids: those belong to +// the server) +type ResourceCollections = Record>; + +export const toResourceCollections = ( + collections: l.CollectionState[] = [] +): ResourceCollections => + collections.reduce((obj: ResourceCollections, c) => { + obj[c.name] = {}; + return obj; + }, {}); + +// Also accepts a plain list of names, which is how collections used to be +// written in openfn.yaml +export const fromResourceCollections = ( + collections: ResourceCollections | string[] | null | undefined +): l.CollectionState[] => + (Array.isArray(collections) + ? collections + : Object.keys(collections ?? {}) + ).map((name) => ({ name })); + +// Build the contents of resources.yaml. Channels and collections are each only +// written if the project knows about them: a missing key means they are not +// managed locally +export const toResources = (project: Project) => { + const resources: Record = {}; + if (project.channels) { + resources.channels = toResourceChannels( + project.channels, + project.credentials + ); + } + if (project.collections) { + resources.collections = toResourceCollections(project.collections); + } + return Object.keys(resources).length ? jsonToYaml(resources) : undefined; +}; + +// Merge a project's collections into an existing resources.yaml, leaving +// everything else in the file (like channels) alone +export const updateResourceCollections = ( + project: Project, + existing?: string +) => { + if (!project.collections) { + return existing; + } + const resources = (existing && yamlToJson(existing)) || {}; + resources.collections = toResourceCollections(project.collections); + return jsonToYaml(resources); +}; diff --git a/packages/project/test/merge/merge-project.test.ts b/packages/project/test/merge/merge-project.test.ts index 2733dfd16..1a92f3ce0 100644 --- a/packages/project/test/merge/merge-project.test.ts +++ b/packages/project/test/merge/merge-project.test.ts @@ -345,7 +345,7 @@ test('replace mode: merged collections keep target uuid on a name match', (t) => t.falsy(result.collections?.find((c) => c.name === 'remove-me')); }); -test('replace mode: no local collections means none survive the merge', (t) => { +test('replace mode: an empty local collections list means none survive the merge', (t) => { const wf = { steps: [ { id: 'x', name: 'X', adaptor: 'common', expression: 'fn(s => s)' }, @@ -357,13 +357,33 @@ test('replace mode: no local collections means none survive the merge', (t) => { const targetCollections = [{ uuid: 'remote-uuid-1', name: 'my-collection' }]; const target = createProject(wf_a, 'a', { collections: targetCollections }); - const source = createProject(wf_b, 'b'); + const source = createProject(wf_b, 'b', { collections: [] }); const result = merge(source, target, { mode: REPLACE_MERGE }); t.deepEqual(result.collections, []); }); +test('replace mode: undefined local collections leave the target collections alone', (t) => { + const wf = { + steps: [ + { id: 'x', name: 'X', adaptor: 'common', expression: 'fn(s => s)' }, + ], + }; + const wf_a = assignUUIDs(wf); + const wf_b = assignUUIDs(wf); + + const targetCollections = [{ uuid: 'remote-uuid-1', name: 'my-collection' }]; + + const target = createProject(wf_a, 'a', { collections: targetCollections }); + // no collections key means they're not managed locally + const source = createProject(wf_b, 'b'); + + const result = merge(source, target, { mode: REPLACE_MERGE }); + + t.deepEqual(result.collections, targetCollections); +}); + test('sandbox mode: source collections fully replace target collections', (t) => { const wf = { steps: [ diff --git a/packages/project/test/parse/from-fs.test.ts b/packages/project/test/parse/from-fs.test.ts index cea19d373..8cd1b3c8a 100644 --- a/packages/project/test/parse/from-fs.test.ts +++ b/packages/project/test/parse/from-fs.test.ts @@ -1,7 +1,7 @@ import test from 'ava'; import mock from 'mock-fs'; import { parseProject } from '../../src/parse/from-fs'; -import { jsonToYaml } from '../../src/util/yaml'; +import { jsonToYaml, yamlToJson } from '../../src/util/yaml'; import { buildConfig } from '../../src/util/config'; test.afterEach(() => { @@ -191,7 +191,29 @@ test.serial('should load the name from config', async (t) => { t.is(project.id, 'peaches-for-me'); }); -test.serial('should load collections from config', async (t) => { +test.serial( + 'legacy collections move to resources.yaml when written back out', + async (t) => { + mockFile('/ws/openfn.yaml', { + workspace: buildConfig(), + project: { id: 'my-project', collections: ['my-collection'] }, + }); + mockFile('/ws/workflows/workflow-1/workflow-1.yaml', { + id: 'workflow-1', + name: 'Workflow 1', + }); + + const project = await parseProject({ root: '/ws' }); + const written = project.serialize('fs'); + + t.deepEqual(yamlToJson(written['resources.yaml']), { + collections: { 'my-collection': {} }, + }); + t.false(written['openfn.yaml'].includes('collections')); + } +); + +test.serial('should load legacy collections from openfn.yaml', async (t) => { mockFile('/ws/openfn.yaml', { workspace: buildConfig(), project: { @@ -217,6 +239,61 @@ test.serial('should load collections from config', async (t) => { t.falsy(project.openfn!.collections); }); +test.serial('should load collections from resources.yaml', async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + mockFile('/ws/resources.yaml', { + collections: { 'my-collection': {}, 'another-collection': {} }, + }); + + const project = await parseProject({ root: '/ws' }); + // in-memory shape is {name, uuid?}, resources.yaml itself has no uuids + t.deepEqual(project.collections, [ + { name: 'my-collection' }, + { name: 'another-collection' }, + ]); +}); + +test.serial( + 'resources.yaml collections take precedence over legacy', + async (t) => { + mockFile('/ws/openfn.yaml', { + workspace: buildConfig(), + project: { id: 'my-project', collections: ['legacy-collection'] }, + }); + mockFile('/ws/resources.yaml', { collections: { 'new-collection': {} } }); + + const project = await parseProject({ root: '/ws' }); + t.deepEqual(project.collections, [{ name: 'new-collection' }]); + } +); + +test.serial( + 'an empty collections key in resources.yaml means no collections', + async (t) => { + mockFile('/ws/openfn.yaml', { + workspace: buildConfig(), + project: { id: 'my-project', collections: ['legacy-collection'] }, + }); + mockFile('/ws/resources.yaml', { collections: {} }); + + const project = await parseProject({ root: '/ws' }); + t.deepEqual(project.collections, []); + } +); + +test.serial( + 'should leave collections undefined if resources.yaml has no collections key', + async (t) => { + mockFile('/ws/openfn.yaml', buildConfig()); + mockFile('/ws/resources.yaml', { + channels: { c: { name: 'c', destination_url: 'https://x.org' } }, + }); + + const project = await parseProject({ root: '/ws' }); + t.is(project.collections, undefined); + } +); + test.serial( 'should return undefined collections when none are configured', async (t) => { diff --git a/packages/project/test/serialize/to-fs.test.ts b/packages/project/test/serialize/to-fs.test.ts index b495afef2..d47277ff0 100644 --- a/packages/project/test/serialize/to-fs.test.ts +++ b/packages/project/test/serialize/to-fs.test.ts @@ -306,6 +306,7 @@ test('toFs: extract a project with 1 workflow and 1 step', (t) => { // Ensure that all the right files have been created t.deepEqual(Object.keys(files), [ 'openfn.json', + '.openfn/checkout.yaml', 'workflows/my-workflow/my-workflow.json', 'workflows/my-workflow/step.js', ]); @@ -314,15 +315,15 @@ test('toFs: extract a project with 1 workflow and 1 step', (t) => { // (this should be validated in more detail by each step) const config = JSON.parse(files['openfn.json']); t.deepEqual(config, { - workspace: { - credentials: 'credentials.yaml', - formats: { openfn: 'json', project: 'yaml', workflow: 'json' }, - dirs: { projects: '.projects', workflows: 'workflows' }, - }, - project: { - id: 'my-project', - name: 'My Project', - }, + credentials: 'credentials.yaml', + dirs: { projects: '.projects', workflows: 'workflows' }, + formats: { openfn: 'json', project: 'yaml', workflow: 'json' }, + }); + + const checkout = yamlToJson(files['.openfn/checkout.yaml']); + t.deepEqual(checkout, { + id: 'my-project', + name: 'My Project', }); const workflow = JSON.parse(files['workflows/my-workflow/my-workflow.json']); @@ -359,6 +360,7 @@ test('toFs: extract a project with forked_from meta', (t) => { // Ensure that all the right files have been created t.deepEqual(Object.keys(files), [ 'openfn.json', + '.openfn/checkout.yaml', 'workflows/my-workflow/my-workflow.json', 'workflows/my-workflow/step.js', ]); @@ -367,16 +369,16 @@ test('toFs: extract a project with forked_from meta', (t) => { // (this should be validated in more detail by each step) const config = JSON.parse(files['openfn.json']); t.deepEqual(config, { - workspace: { - credentials: 'credentials.yaml', - formats: { openfn: 'json', project: 'yaml', workflow: 'json' }, - dirs: { projects: '.projects', workflows: 'workflows' }, - }, - project: { - id: 'my-project', - name: 'My Project', - forked_from: 'abcd', - }, + credentials: 'credentials.yaml', + dirs: { projects: '.projects', workflows: 'workflows' }, + formats: { openfn: 'json', project: 'yaml', workflow: 'json' }, + }); + + const checkout = yamlToJson(files['.openfn/checkout.yaml']); + t.deepEqual(checkout, { + id: 'my-project', + name: 'My Project', + forked_from: 'abcd', }); const workflow = JSON.parse(files['workflows/my-workflow/my-workflow.json']); @@ -386,17 +388,12 @@ test('toFs: extract a project with forked_from meta', (t) => { t.is(files['workflows/my-workflow/step.js'], 'fn(s => s)'); }); -test('toFs: writes collection names into openfn.json (freshly authored, no uuid yet)', (t) => { +test('toFs: writes collections to resources.yaml keyed by name (freshly authored, no uuid yet)', (t) => { const project = new Project( { name: 'My Project', collections: [{ name: 'my-collection' }, { name: 'another-collection' }], - workflows: [ - { - id: 'my-workflow', - steps: [step], - }, - ], + workflows: [], }, { formats: { @@ -408,70 +405,67 @@ test('toFs: writes collection names into openfn.json (freshly authored, no uuid const files = toFs(project); - const config = JSON.parse(files['openfn.json']); - t.deepEqual(config.project.collections, [ - 'my-collection', - 'another-collection', - ]); + t.deepEqual(yamlToJson(files['resources.yaml']), { + collections: { 'my-collection': {}, 'another-collection': {} }, + }); + // collections don't belong in openfn.json + t.falsy(JSON.parse(files['openfn.json']).collections); }); -test('toFs: strips uuids from fetched collections when writing to openfn.json', (t) => { - const project = new Project( - { - name: 'My Project', - // this is the shape a freshly-fetched project's collections are in - - // uuids should never be written to openfn.yaml - collections: [ - { uuid: 'remote-uuid-1', name: 'my-collection' }, - { uuid: 'remote-uuid-2', name: 'another-collection' }, - ], - workflows: [ - { - id: 'my-workflow', - steps: [step], - }, - ], - }, - { - formats: { - openfn: 'json', - workflow: 'json', - }, - } - ); +test('toFs: strips uuids from fetched collections when writing to resources.yaml', (t) => { + const project = new Project({ + name: 'My Project', + // this is the shape a freshly-fetched project's collections are in - + // uuids should never be written to disk + collections: [ + { uuid: 'remote-uuid-1', name: 'my-collection' }, + { uuid: 'remote-uuid-2', name: 'another-collection' }, + ], + workflows: [], + }); const files = toFs(project); - const config = JSON.parse(files['openfn.json']); - t.deepEqual(config.project.collections, [ - 'my-collection', - 'another-collection', - ]); + t.deepEqual(yamlToJson(files['resources.yaml']), { + collections: { 'my-collection': {}, 'another-collection': {} }, + }); }); -test('toFs: omits collections key when there are none', (t) => { - const project = new Project( - { - name: 'My Project', - workflows: [ - { - id: 'my-workflow', - steps: [step], - }, - ], - }, - { - formats: { - openfn: 'json', - workflow: 'json', - }, - } - ); +test('toFs: writes an empty collections key when the project has none', (t) => { + const project = new Project({ + name: 'My Project', + collections: [], + workflows: [], + }); const files = toFs(project); - const config = JSON.parse(files['openfn.json']); - t.falsy(config.project.collections); + // an empty key means "no collections", which is not the same as no key + t.deepEqual(yamlToJson(files['resources.yaml']), { collections: {} }); +}); + +test('toFs: writes collections and channels to the same resources.yaml', (t) => { + const project = new Project({ + name: 'My Project', + collections: [{ name: 'my-collection' }], + channels: [ + { + name: 'My Channel', + destination_url: 'https://example.com', + enabled: true, + }, + ], + workflows: [], + }); + + const resources = yamlToJson(toFs(project)['resources.yaml']); + t.deepEqual(Object.keys(resources).sort(), ['channels', 'collections']); +}); + +test('toFs: does not write resources.yaml when collections are unknown', (t) => { + const project = new Project({ name: 'My Project', workflows: [] }); + + t.false('resources.yaml' in toFs(project)); }); test('toFs: writes channels to resources.yaml keyed by id, with credential names', (t) => { diff --git a/packages/project/test/util/config.test.ts b/packages/project/test/util/config.test.ts index 02cdb5ffd..6866e406f 100644 --- a/packages/project/test/util/config.test.ts +++ b/packages/project/test/util/config.test.ts @@ -1,6 +1,9 @@ import test from 'ava'; import { + extractCheckout, extractConfig, + getCheckoutPath, + hasCheckoutMeta, findWorkspaceFile, loadWorkspaceFile, } from '../../src/util/config'; @@ -158,7 +161,7 @@ test('find openfn.json', (t) => { t.deepEqual(result.content, { x: 1 }); }); -test('generate openfn.yaml', (t) => { +test('generate openfn.yaml (workspace config only)', (t) => { const proj = new Project( { id: 'my-project', @@ -177,28 +180,65 @@ test('generate openfn.yaml', (t) => { } ); const result = extractConfig(proj); - t.is(result.path, 'openfn.yaml'), - t.deepEqual( - result.content, - `project: - uuid: 1234 - id: my-project - name: My Project - forked_from: abcd -workspace: - credentials: credentials.yaml - formats: - openfn: yaml - project: yaml - workflow: yaml - dirs: - projects: .projects - workflows: workflows + t.is(result.path, 'openfn.yaml'); + // keys are sorted alphabetically, and there's no project block + t.deepEqual( + result.content, + `credentials: credentials.yaml +dirs: + projects: .projects + workflows: workflows +formats: + openfn: yaml + project: yaml + workflow: yaml ` - ); + ); +}); + +test('generate openfn.yaml does not include collections', (t) => { + const proj = new Project({ + id: 'my-project', + collections: [{ uuid: 'remote-uuid', name: 'my-collection' }], + }); + const result = extractConfig(proj); + t.false(result.content.includes('collections')); }); -test("exclude forked_from if it's not set", (t) => { +test('generate checkout file with forked_from', (t) => { + const proj = new Project({ + id: 'my-project', + name: 'My Project', + openfn: { + uuid: 1234, + }, + cli: { + forked_from: 'abcd', + }, + }); + const result = extractCheckout(proj); + t.is(result.path, '.openfn/checkout.yaml'); + t.deepEqual( + result.content, + `forked_from: abcd +id: my-project +name: My Project +uuid: 1234 +` + ); +}); + +test('getCheckoutPath: default and per-branch', (t) => { + t.is(getCheckoutPath(), '.openfn/checkout.yaml'); + t.is(getCheckoutPath(false), '.openfn/checkout.yaml'); + t.is(getCheckoutPath('dev'), '.openfn/branches/dev/checkout.yaml'); + t.is( + getCheckoutPath('feature/x'), + '.openfn/branches/feature/x/checkout.yaml' + ); +}); + +test("checkout file excludes forked_from if it's not set", (t) => { const proj = new Project( { id: 'my-project', @@ -214,25 +254,15 @@ test("exclude forked_from if it's not set", (t) => { }, } ); - const result = extractConfig(proj); - t.is(result.path, 'openfn.yaml'), - t.deepEqual( - result.content, - `project: - uuid: 1234 - id: my-project - name: My Project -workspace: - credentials: credentials.yaml - formats: - openfn: yaml - project: yaml - workflow: yaml - dirs: - projects: .projects - workflows: workflows + const result = extractCheckout(proj, 'dev'); + t.is(result.path, '.openfn/branches/dev/checkout.yaml'); + t.deepEqual( + result.content, + `id: my-project +name: My Project +uuid: 1234 ` - ); + ); }); test.todo('generate openfn.json'); @@ -242,9 +272,59 @@ test('include project name', (t) => { id: 'my-project', name: 'My Project', }); - const result = extractConfig(proj, 'json'); - const json = JSON.parse(result.content); - t.is(json.project.name, 'My Project'); + const result = extractCheckout(proj); + t.true(result.content.includes('name: My Project')); }); test.todo('include parent project name'); + +test('hasCheckoutMeta: false if there is no workspace at all', (t) => { + mock({ '/ws': {} }); + + t.false(hasCheckoutMeta('/ws')); +}); + +test('hasCheckoutMeta: false if there is a workspace but no checkout file', (t) => { + mock({ '/ws/openfn.yaml': 'credentials: creds.yaml' }); + + t.false(hasCheckoutMeta('/ws')); +}); + +test('hasCheckoutMeta: true if there is a checkout file', (t) => { + mock({ + '/ws/openfn.yaml': 'credentials: creds.yaml', + '/ws/.openfn/checkout.yaml': 'id: my-project', + }); + + t.true(hasCheckoutMeta('/ws')); +}); + +test('hasCheckoutMeta: false if the checkout file is empty', (t) => { + mock({ + '/ws/openfn.yaml': 'credentials: creds.yaml', + '/ws/.openfn/checkout.yaml': '', + }); + + t.false(hasCheckoutMeta('/ws')); +}); + +test('hasCheckoutMeta: true if there is a legacy project block in openfn.yaml', (t) => { + mock({ + '/ws/openfn.yaml': + 'project:\n id: my-project\nworkspace:\n credentials: creds.yaml', + }); + + t.true(hasCheckoutMeta('/ws')); +}); + +test('hasCheckoutMeta: looks for the checkout file for the given branch', (t) => { + mock({ + '/ws/openfn.yaml': 'credentials: creds.yaml', + '/ws/.openfn/branches/dev/checkout.yaml': 'id: my-project', + }); + + t.true(hasCheckoutMeta('/ws', 'dev')); + // other branches, and no branch, don't have any + t.false(hasCheckoutMeta('/ws', 'main')); + t.false(hasCheckoutMeta('/ws')); +}); diff --git a/packages/project/test/util/project-diff.test.ts b/packages/project/test/util/project-diff.test.ts index 5aa48d9f8..509527761 100644 --- a/packages/project/test/util/project-diff.test.ts +++ b/packages/project/test/util/project-diff.test.ts @@ -321,3 +321,45 @@ test('diff: should report workflow and channel changes together', (t) => { t.is(result.workflows.length, 1); t.is(result.resources.channels.length, 1); }); + +const withCollections = (collections?: string[]) => + new Project({ + name: 'p', + workflows: [], + collections: collections?.map((name) => ({ name })), + }); + +test('diff: resources.collections is empty for identical collections', (t) => { + const a = withCollections(['one']); + const b = withCollections(['one']); + + t.deepEqual(diff(a, b).resources.collections, []); +}); + +test('diff: should detect added and removed collections', (t) => { + const a = withCollections(['keep', 'remove']); + const b = withCollections(['keep', 'add']); + + t.deepEqual(diff(a, b).resources.collections, [ + { name: 'remove', type: 'removed' }, + { name: 'add', type: 'added' }, + ]); +}); + +test('diff: should report all collections removed when one side is emptied', (t) => { + const remote = withCollections(['one', 'two']); + const local = withCollections([]); + + t.deepEqual(diff(remote, local).resources.collections, [ + { name: 'one', type: 'removed' }, + { name: 'two', type: 'removed' }, + ]); +}); + +test('diff: should ignore collections if either project does not define them', (t) => { + const withOne = withCollections(['one']); + const none = withCollections(undefined); + + t.deepEqual(diff(withOne, none).resources.collections, []); + t.deepEqual(diff(none, withOne).resources.collections, []); +}); diff --git a/packages/project/test/util/resources.test.ts b/packages/project/test/util/resources.test.ts new file mode 100644 index 000000000..62dae7c83 --- /dev/null +++ b/packages/project/test/util/resources.test.ts @@ -0,0 +1,65 @@ +import test from 'ava'; +import { Project } from '../../src/Project'; +import { jsonToYaml, yamlToJson } from '../../src/util/yaml'; +import { + fromResourceCollections, + toResourceCollections, + updateResourceCollections, +} from '../../src/util/resources'; + +test('toResourceCollections: keyed by name, with no uuids', (t) => { + t.deepEqual( + toResourceCollections([{ uuid: 'a', name: 'one' }, { name: 'two' }]), + { one: {}, two: {} } + ); +}); + +test('fromResourceCollections: reads a map keyed by name', (t) => { + t.deepEqual(fromResourceCollections({ one: {}, two: {} }), [ + { name: 'one' }, + { name: 'two' }, + ]); +}); + +test('fromResourceCollections: also reads a plain list of names', (t) => { + t.deepEqual(fromResourceCollections(['one', 'two']), [ + { name: 'one' }, + { name: 'two' }, + ]); +}); + +test('fromResourceCollections: an empty or null key means no collections', (t) => { + t.deepEqual(fromResourceCollections({}), []); + t.deepEqual(fromResourceCollections(null), []); +}); + +test('updateResourceCollections: replaces collections and leaves other keys alone', (t) => { + const project = new Project({ name: 'p', collections: [{ name: 'new' }] }); + const existing = jsonToYaml({ + channels: { c: { name: 'c', destination_url: 'https://x.org' } }, + collections: { old: {} }, + }); + + const result = yamlToJson(updateResourceCollections(project, existing)!); + + t.deepEqual(result, { + channels: { c: { name: 'c', destination_url: 'https://x.org' } }, + collections: { new: {} }, + }); +}); + +test('updateResourceCollections: creates the content if there is no existing file', (t) => { + const project = new Project({ name: 'p', collections: [{ name: 'one' }] }); + + t.deepEqual(yamlToJson(updateResourceCollections(project)!), { + collections: { one: {} }, + }); +}); + +test('updateResourceCollections: leaves the file alone if collections are not managed', (t) => { + const project = new Project({ name: 'p' }); + const existing = jsonToYaml({ channels: {} }); + + t.is(updateResourceCollections(project, existing), existing); + t.is(updateResourceCollections(project), undefined); +}); From e90bfde7599935ba86f43bf63255e915966118ae Mon Sep 17 00:00:00 2001 From: Midigo Frank <39288959+midigofrank@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:24:46 +0300 Subject: [PATCH 2/7] CLI: handle multiple local copies of the same project (#1575) * project: tell local copies of the same project apart by alias A workspace can hold several local copies of one project, each from its own state file (main@app.openfn.org.yaml, backup@app.openfn.org.yaml). They share a UUID and id, so anything keyed on those mixed them up. Copies overwrote each other's entries in the path map, the checkout had no way to say which copy was checked out, and get() threw even when given an exact alias. matchProject now returns the exact alias match when all the other matches share its UUID. Real ambiguity still throws, and the error lists each clash as alias@host and suggests using one. Workspace keeps project paths by Project rather than by id, so each copy keeps its own file. getProjectPath() now takes a Project. extractCheckout writes the active project's alias to the checkout file, and getTrackedProject() looks that alias and host up first. The alias is left out of project.openfn so the metadata doesn't store it twice. merge used getProjectPath(id) to find where to write. With --base it only got the right file because the id-keyed map happened to point there. It now writes back to the --base file directly. * cli: don't fail fetch on duplicate local projects when the target is explicit `openfn project fetch --endpoint X -o file.yaml` threw MultipleMatchingProjectsError if two local project files shared the UUID. With a full UUID, an endpoint and an output path, the command has everything it needs, so the local copies don't matter. Skip the local workspace lookup in that case. Also stop searching the workspace for an output target once -o is given, because the path is the target. Duplicates still throw when the UUID, endpoint or output path is missing. * cli: use the checked-out copy for pull and deploy without a target When no project was given, pull and deploy fell back to the active project's UUID. That throws if another local copy shares the UUID, even though the checkout file says which one is checked out. If the checkout file records an alias, pull now defaults to the tracked project's alias@host and deploy uses the tracked project. Without an alias, both fall back to the UUID as before. * add changeset * address review feedback getTrackedProject now tries alias, uuid then id in plain steps, and matches the alias on the full endpoint origin, so copies on different ports of the same host (localhost:4000, localhost:5000) are told apart. deploy picks its sync target in one place: an explicit target, the local copy of a deployed file, or the tracked project. The ambiguity error now suggests alias@domain or a path. Adds a test for fetching by alias when local copies share a UUID. * little tidy --------- Co-authored-by: Joe Clark --- .changeset/quiet-copies-match.md | 6 + packages/cli/src/projects/deploy.ts | 14 ++- packages/cli/src/projects/fetch.ts | 12 +- packages/cli/src/projects/merge.ts | 6 +- packages/cli/src/projects/pull.ts | 5 +- packages/cli/test/projects/checkout.test.ts | 3 +- packages/cli/test/projects/fetch.test.ts | 106 ++++++++++++++++++ packages/cli/test/projects/util.test.ts | 1 + packages/project/src/Workspace.ts | 40 +++++-- packages/project/src/parse/from-fs.ts | 2 +- packages/project/src/util/config.ts | 4 + packages/project/src/util/match-project.ts | 37 +++--- packages/project/test/serialize/to-fs.test.ts | 2 + packages/project/test/util/config.test.ts | 6 +- .../project/test/util/match-project.test.ts | 11 ++ packages/project/test/workspace.test.ts | 69 ++++++++++++ 16 files changed, 282 insertions(+), 42 deletions(-) create mode 100644 .changeset/quiet-copies-match.md diff --git a/.changeset/quiet-copies-match.md b/.changeset/quiet-copies-match.md new file mode 100644 index 000000000..87f270a4a --- /dev/null +++ b/.changeset/quiet-copies-match.md @@ -0,0 +1,6 @@ +--- +'@openfn/cli': patch +'@openfn/project': patch +--- + +Handle multiple local copies of the same project without reporting id conflicts diff --git a/packages/cli/src/projects/deploy.ts b/packages/cli/src/projects/deploy.ts index 88c4fd8f3..1b5af951f 100644 --- a/packages/cli/src/projects/deploy.ts +++ b/packages/cli/src/projects/deploy.ts @@ -446,12 +446,18 @@ export async function handler(options: DeployOptions, logger: Logger) { ws ??= new Workspace(options.workspace || '.', undefined, true, { branch: options.branch, }); - tracker = ws.get(targetIdentifier ?? localProject.uuid!); + if (targetIdentifier) { + tracker = ws.get(targetIdentifier); + } else if (filePath) { + tracker = ws.get(localProject.uuid!); + } else { + tracker = ws.getTrackedProject(); + } // A project loaded from a file already knows which remote it belongs - // to, so it can serve as its own deploy destination - we don't need a - // locally tracked copy of it to sync against - if (!tracker && filePath && localProject.uuid) { + // to, so it can serve as its own deploy destination if there's no + // locally tracked copy of it + if (!tracker && filePath) { logger.debug( 'No locally tracked project found: deploying to the remote named in the file' ); diff --git a/packages/cli/src/projects/fetch.ts b/packages/cli/src/projects/fetch.ts index 0808cfe9e..64ed56bad 100644 --- a/packages/cli/src/projects/fetch.ts +++ b/packages/cli/src/projects/fetch.ts @@ -208,6 +208,8 @@ async function resolveOutputProject( } catch (e) { logger.debug('No project found at', options.outputPath); } + // An explicit path is the target: don't look in the workspace + return; } // if an alias is specified, we use that as the output if (options.alias) { @@ -254,7 +256,12 @@ export async function fetchRemoteProject( // First, we need to see if the project argument, which might be a UUID, id or alias, // resolves to anything - const localProject = workspace.get(options.project!); + // A full UUID, endpoint and output path leave nothing to resolve locally, + // so don't trip over duplicate local copies + const localProject = + options.outputPath && options.endpoint && UUID_RE.test(options.project!) + ? undefined + : workspace.get(options.project!); if ( localProject?.openfn?.uuid && localProject.openfn.uuid !== options.project @@ -303,6 +310,9 @@ export async function fetchRemoteProject( return project; } +const UUID_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + function ensureTargetCompatible( options: FetchOptions, remoteProject: Project, diff --git a/packages/cli/src/projects/merge.ts b/packages/cli/src/projects/merge.ts index 08634ce06..47471a7fe 100644 --- a/packages/cli/src/projects/merge.ts +++ b/packages/cli/src/projects/merge.ts @@ -76,8 +76,8 @@ export const handler = async (options: MergeOptions, logger: Logger) => { } let targetProject: Project; - if (options.base) { - const basePath = path.resolve(options.base); + const basePath = options.base && path.resolve(options.base); + if (basePath) { logger.debug('Loading target project from path', basePath); targetProject = await Project.from('path', basePath); } else { @@ -143,7 +143,7 @@ export const handler = async (options: MergeOptions, logger: Logger) => { } const finalPath = - options.outputPath ?? workspace.getProjectPath(targetProject.id); + options.outputPath || basePath || workspace.getProjectPath(targetProject); if (!finalPath) { logger.error('Path to checked out project not found.'); return; diff --git a/packages/cli/src/projects/pull.ts b/packages/cli/src/projects/pull.ts index 06d183ef2..faec0baa9 100644 --- a/packages/cli/src/projects/pull.ts +++ b/packages/cli/src/projects/pull.ts @@ -89,7 +89,10 @@ const ensureProjectId = async (options: any, logger: Logger) => { logger ); if (ws.activeProject) { - options.project = ws.activeProject.uuid; + // Prefer alias@domain: other local copies may share the uuid + options.project = + (ws.activeProject.alias && ws.getTrackedProject()?.qname) || + ws.activeProject.uuid; logger?.info( `Project id not provided: will default to ${options.project}` ); diff --git a/packages/cli/test/projects/checkout.test.ts b/packages/cli/test/projects/checkout.test.ts index fb6d69e7a..7c190e305 100644 --- a/packages/cli/test/projects/checkout.test.ts +++ b/packages/cli/test/projects/checkout.test.ts @@ -477,7 +477,8 @@ formats: const checkout = fs.readFileSync('/ws1/.openfn/checkout.yaml', 'utf8'); t.is( checkout, - `id: staging + `alias: staging +id: staging name: Staging uuid: ` diff --git a/packages/cli/test/projects/fetch.test.ts b/packages/cli/test/projects/fetch.test.ts index 5e227ea47..b65f7b2de 100644 --- a/packages/cli/test/projects/fetch.test.ts +++ b/packages/cli/test/projects/fetch.test.ts @@ -540,3 +540,109 @@ test.serial('force merge a diverged project', async (t) => { // The file should be overwritten t.regex(fileContent, /fn\(\)/); }); + +test.serial( + 'fetch by UUID to an output path when local copies share the UUID', + async (t) => { + mock({ + '/ws/.projects/jam@app.openfn.org.yaml': yaml_v1, + '/ws/.projects/jam-backup@app.openfn.org.yaml': yaml_v1, + '/ws/openfn.yaml': '', + }); + + await fetchHandler( + { + project: PROJECT_UUID, + outputPath: '/ws/out.yaml', + + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + } as any, + logger + ); + + const fileContent = await readFile('/ws/out.yaml', 'utf-8'); + t.is(fileContent.trim(), yaml_v2); + } +); + +test.serial( + 'error: fetch by UUID to an output path without an endpoint when local copies share the UUID', + async (t) => { + mock({ + '/ws/.projects/jam@app.openfn.org.yaml': yaml_v1, + '/ws/.projects/jam-backup@app.openfn.org.yaml': yaml_v1, + '/ws/openfn.yaml': '', + }); + + await t.throwsAsync( + () => + fetchHandler( + { + project: PROJECT_UUID, + outputPath: '/ws/out.yaml', + + apiKey: 'test-api-key', + workspace: '/ws', + } as any, + logger + ), + { message: /Failed to resolve unique identifier/ } + ); + } +); + +test.serial( + 'error: fetch by UUID without an output path when local copies share the UUID', + async (t) => { + mock({ + '/ws/.projects/jam@app.openfn.org.yaml': yaml_v1, + '/ws/.projects/jam-backup@app.openfn.org.yaml': yaml_v1, + '/ws/openfn.yaml': '', + }); + + await t.throwsAsync( + () => + fetchHandler( + { + project: PROJECT_UUID, + + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + } as any, + logger + ), + { message: /Failed to resolve unique identifier/ } + ); + } +); + +test.serial('fetch by alias when local copies share the UUID', async (t) => { + mock({ + '/ws/.projects/jam@app.openfn.org.yaml': yaml_v1, + '/ws/.projects/jam-backup@app.openfn.org.yaml': yaml_v1, + '/ws/openfn.yaml': '', + }); + + await fetchHandler( + { + project: 'jam-backup', + + endpoint: ENDPOINT, + apiKey: 'test-api-key', + workspace: '/ws', + } as any, + logger + ); + + // Only the aliased copy is updated + const backup = await readFile( + '/ws/.projects/jam-backup@app.openfn.org.yaml', + 'utf-8' + ); + t.is(backup.trim(), yaml_v2); + const main = await readFile('/ws/.projects/jam@app.openfn.org.yaml', 'utf-8'); + t.is(main, yaml_v1); +}); diff --git a/packages/cli/test/projects/util.test.ts b/packages/cli/test/projects/util.test.ts index 3f5b36ce0..453e92d31 100644 --- a/packages/cli/test/projects/util.test.ts +++ b/packages/cli/test/projects/util.test.ts @@ -299,6 +299,7 @@ test.serial( t.is(filePath, '/ws/.openfn/checkout.yaml'); t.deepEqual(yamlToJson(fs.readFileSync(filePath, 'utf8')), { + alias: 'main', endpoint: 'https://app.openfn.org', forked_from: { wf: 'latest' }, id: 'my-project', diff --git a/packages/project/src/Workspace.ts b/packages/project/src/Workspace.ts index 018b21a78..d86878fc5 100644 --- a/packages/project/src/Workspace.ts +++ b/packages/project/src/Workspace.ts @@ -34,7 +34,7 @@ export class Workspace { branch?: string | false | null; private projects: Project[] = []; - private projectPaths = new Map(); + private projectPaths = new Map(); private isValid: boolean = false; private logger: Logger; @@ -97,7 +97,7 @@ export class Workspace { ...this.config, alias, }); - this.projectPaths.set(project.id, stateFilePath); + this.projectPaths.set(project, stateFilePath); return project; } catch (e) { console.warn(`Failed to load project from ${stateFilePath}`); @@ -138,15 +138,36 @@ export class Workspace { return matchProject(nameyThing, this.projects); } - getProjectPath(id: string) { - return this.projectPaths.get(id); + getProjectPath(project: Project) { + return this.projectPaths.get(project); } + /** + * Find the local copy of the checked-out project. + * + * Several local copies can share a uuid and id (eg main@app.openfn.org and + * staging@localhost), so the checkout's alias and endpoint are tried first. + * Older checkouts may not record an alias: fall back to uuid, then id. + */ getTrackedProject() { - return ( - this.projects.find((p) => p.openfn?.uuid === this.activeProject?.uuid) ?? - this.projects.find((p) => p.id === this.activeProject?.id) - ); + const { alias, endpoint, uuid, id } = this.activeProject ?? {}; + + if (alias) { + const match = this.projects.find( + (p) => + p.alias === alias && + // Compare origins (not just hostnames) so that ports count + (!endpoint || sameOrigin(p.openfn?.endpoint, endpoint)) + ); + if (match) return match; + } + + if (uuid) { + const match = this.projects.find((p) => p.openfn?.uuid === uuid); + if (match) return match; + } + + return this.projects.find((p) => p.id === id); } async getCheckedOutProject(alias?: string | null) { @@ -181,3 +202,6 @@ export class Workspace { return this.isValid; } } + +const sameOrigin = (a?: string, b?: string) => + !!a && !!b && new URL(a).origin === new URL(b).origin; diff --git a/packages/project/src/parse/from-fs.ts b/packages/project/src/parse/from-fs.ts index e104702f4..6235a3ab4 100644 --- a/packages/project/src/parse/from-fs.ts +++ b/packages/project/src/parse/from-fs.ts @@ -47,7 +47,7 @@ export const parseProject = async (options: FromFsConfig) => { const proj: any = { id: options.name ? slugify(options.name) : checkout.id, name: options.name ? slugify(options.name) : checkout.name, - openfn: omit(checkout, ['id', 'forked_from', 'collections']), + openfn: omit(checkout, ['id', 'alias', 'forked_from', 'collections']), // Legacy: collections used to be a list of names in openfn.yaml. A // collections key in resources.yaml takes precedence (see below) diff --git a/packages/project/src/util/config.ts b/packages/project/src/util/config.ts index e4fe7dbe7..b4801e834 100644 --- a/packages/project/src/util/config.ts +++ b/packages/project/src/util/config.ts @@ -63,6 +63,10 @@ export const extractCheckout = ( project.name = source.name; } + if (source.alias) { + project.alias = source.alias; + } + if (source.cli.forked_from && Object.keys(source.cli.forked_from).length) { project.forked_from = source.cli.forked_from; } diff --git a/packages/project/src/util/match-project.ts b/packages/project/src/util/match-project.ts index 1032df5ab..0c5be62f7 100644 --- a/packages/project/src/util/match-project.ts +++ b/packages/project/src/util/match-project.ts @@ -9,35 +9,30 @@ export class MultipleMatchingProjectsError extends Error {} const matchProject = (name: Alias | ID | UUID, candidates: Project[]) => { const [searchTerm, domain] = `${name}`.split('@'); - // Collect all matching projects - const matchingProjects: Record = {}; - let multipleIdMatches = false; - - // Filter candidates by domain - candidates = candidates.filter( - (project) => !domain || project.host === domain + const re = new RegExp(searchTerm, 'i'); + const matches = candidates.filter( + (project) => + (!domain || project.host === domain) && + (project.id === searchTerm || + project.alias === searchTerm || + (project.uuid && re.test(project.uuid))) ); - const re = new RegExp(searchTerm, 'i'); - for (const project of candidates) { + if (matches.length > 1) { + // Aliases come from file names, so an exact alias match picks out one + // local copy of a project, even if other copies share its id or uuid + const aliasMatches = matches.filter((p) => p.alias === searchTerm); if ( - project.id === searchTerm || - project.alias === searchTerm || - (project.uuid && re.test(project.uuid)) + aliasMatches.length === 1 && + matches.every((p) => p.uuid === aliasMatches[0].uuid) ) { - matchingProjects[project.id] ??= []; - matchingProjects[project.id].push(project); + return aliasMatches[0]; } - } - - const matches = Object.values(matchingProjects).flat(); - // Multiple matches - throw error - if (multipleIdMatches || matches.length > 1) { throw new MultipleMatchingProjectsError( `Failed to resolve unique identifier for "${name}", clashes with: ${matches - .map((p) => p.id) - .join(', ')}` + .map((p) => p.qname) + .join(', ')}. Use alias@domain or a path instead` ); } return matches.length ? matches[0] : null; diff --git a/packages/project/test/serialize/to-fs.test.ts b/packages/project/test/serialize/to-fs.test.ts index d47277ff0..5f6dd437c 100644 --- a/packages/project/test/serialize/to-fs.test.ts +++ b/packages/project/test/serialize/to-fs.test.ts @@ -322,6 +322,7 @@ test('toFs: extract a project with 1 workflow and 1 step', (t) => { const checkout = yamlToJson(files['.openfn/checkout.yaml']); t.deepEqual(checkout, { + alias: 'main', id: 'my-project', name: 'My Project', }); @@ -376,6 +377,7 @@ test('toFs: extract a project with forked_from meta', (t) => { const checkout = yamlToJson(files['.openfn/checkout.yaml']); t.deepEqual(checkout, { + alias: 'main', id: 'my-project', name: 'My Project', forked_from: 'abcd', diff --git a/packages/project/test/util/config.test.ts b/packages/project/test/util/config.test.ts index 6866e406f..0cd128af3 100644 --- a/packages/project/test/util/config.test.ts +++ b/packages/project/test/util/config.test.ts @@ -220,7 +220,8 @@ test('generate checkout file with forked_from', (t) => { t.is(result.path, '.openfn/checkout.yaml'); t.deepEqual( result.content, - `forked_from: abcd + `alias: main +forked_from: abcd id: my-project name: My Project uuid: 1234 @@ -258,7 +259,8 @@ test("checkout file excludes forked_from if it's not set", (t) => { t.is(result.path, '.openfn/branches/dev/checkout.yaml'); t.deepEqual( result.content, - `id: my-project + `alias: main +id: my-project name: My Project uuid: 1234 ` diff --git a/packages/project/test/util/match-project.test.ts b/packages/project/test/util/match-project.test.ts index 8362f9223..6beb3c705 100644 --- a/packages/project/test/util/match-project.test.ts +++ b/packages/project/test/util/match-project.test.ts @@ -143,6 +143,17 @@ test('throw if ambiguous - id matches one, alias matches another', (t) => { }); }); +test('match a copy by alias when other copies share its id', (t) => { + const projects = [ + p('', 'main', 'my-project'), + p('', 'my-project', 'my-project'), + ]; + + const result = matchProject('my-project', projects); + + t.is(result?.alias, 'my-project'); +}); + test('throw if ambiguous uuid', (t) => { const projects = [ p('abcd1234-5678-90ef-ghij-klmnopqrstuv', 'staging-a', 'project-a'), diff --git a/packages/project/test/workspace.test.ts b/packages/project/test/workspace.test.ts index ec8c6f973..b8ae20410 100644 --- a/packages/project/test/workspace.test.ts +++ b/packages/project/test/workspace.test.ts @@ -191,6 +191,42 @@ mock({ }), // aliasing + // Two local copies of the same project, with "main" checked out + '/ws5/openfn.yaml': jsonToYaml({ + project: { id: 'proj-1', uuid: '111', alias: 'main' }, + }), + '/ws5/.projects/main@openfn.org.yaml': gen( + 111, + 'main', + 'proj-1', + 'openfn.org' + ), + '/ws5/.projects/backup@openfn.org.yaml': gen( + 111, + 'backup', + 'proj-1', + 'openfn.org' + ), + // Two copies on different ports of the same host + '/ws6/openfn.yaml': '', + '/ws6/.openfn/checkout.yaml': jsonToYaml({ + id: 'proj-1', + uuid: '222', + alias: 'local', + endpoint: 'http://localhost:5000', + }), + '/ws6/.projects/local@localhost.yaml': jsonToYaml({ + id: 'proj-1', + version: 2, + openfn: { uuid: '111', endpoint: 'http://localhost:4000' }, + workflows: [], + }), + '/ws6/.projects/other@localhost.yaml': jsonToYaml({ + id: 'proj-1', + version: 2, + openfn: { uuid: '222', endpoint: 'http://localhost:5000' }, + workflows: [], + }), '/ws4/openfn.yaml': '', '/ws4/.projects/main@openfn.org.yaml': gen( 111, @@ -321,3 +357,36 @@ test('get project throws on ambiguous match', (t) => { t.truthy(error); t.regex(error!.message, /Failed to resolve unique identifier/); }); + +test('get a duplicated project by uuid throws', (t) => { + const ws = new Workspace('/ws5'); + + t.throws(() => ws.get('111'), { + message: /Failed to resolve unique identifier/, + }); +}); + +test('getTrackedProject returns the checked out copy by alias', (t) => { + const ws = new Workspace('/ws5'); + + t.is(ws.getTrackedProject()?.alias, 'main'); +}); + +test('getTrackedProject matches the alias on the same port only', (t) => { + const ws = new Workspace('/ws6'); + + t.is(ws.getTrackedProject()?.alias, 'other'); +}); + +test('getProjectPath returns the file for each copy', (t) => { + const ws = new Workspace('/ws5'); + + t.is( + ws.getProjectPath(ws.get('main')!), + '/ws5/.projects/main@openfn.org.yaml' + ); + t.is( + ws.getProjectPath(ws.get('backup')!), + '/ws5/.projects/backup@openfn.org.yaml' + ); +}); From d331af227bc66b11287de28173e6aff8e9b3eb9d Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Fri, 9 Oct 2026 09:42:31 +0100 Subject: [PATCH 3/7] versions --- packages/project/CHANGELOG.md | 15 +++++++++++++++ packages/project/package.json | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/project/CHANGELOG.md b/packages/project/CHANGELOG.md index ec9eac1a6..4d8b598cc 100644 --- a/packages/project/CHANGELOG.md +++ b/packages/project/CHANGELOG.md @@ -1,5 +1,20 @@ # @openfn/project +## 0.22.0 + +### Minor Changes + +- d3f1a1a: Restructure `openfn.yaml` to make it safer for merging on github. + + Any checkout state (basically anything under the project key) have been moved into a new `.openfn` folder, which should not be tracked. + + `openfn.yaml` now just contains workspace-wide configuration. + +### Patch Changes + +- e90bfde: Handle multiple local copies of the same project without reporting id conflicts +- d3f1a1a: Track collections in resources.yaml + ## 0.21.0 ### Minor Changes diff --git a/packages/project/package.json b/packages/project/package.json index f4d1bfa7f..6e73fbad0 100644 --- a/packages/project/package.json +++ b/packages/project/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/project", - "version": "0.21.0", + "version": "0.22.0", "description": "Read, serialize, replicate and sync OpenFn projects", "author": "Open Function Group ", "license": "ISC", From 74dac2be817b78966d57937d7d3bbe5a4649a674 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Fri, 9 Oct 2026 09:43:16 +0100 Subject: [PATCH 4/7] versions --- .changeset/eager-houses-call.md | 10 ---------- .changeset/hip-actors-switch.md | 5 ----- .changeset/orange-lions-talk.md | 5 ----- .changeset/quiet-copies-match.md | 6 ------ .changeset/small-cases-buy.md | 5 ----- .changeset/true-dolls-stay.md | 6 ------ integration-tests/cli/CHANGELOG.md | 10 ++++++++++ integration-tests/cli/package.json | 2 +- packages/cli/CHANGELOG.md | 22 ++++++++++++++++++++++ packages/cli/package.json | 2 +- packages/lightning-mock/CHANGELOG.md | 9 +++++++++ packages/lightning-mock/package.json | 2 +- 12 files changed, 44 insertions(+), 40 deletions(-) delete mode 100644 .changeset/eager-houses-call.md delete mode 100644 .changeset/hip-actors-switch.md delete mode 100644 .changeset/orange-lions-talk.md delete mode 100644 .changeset/quiet-copies-match.md delete mode 100644 .changeset/small-cases-buy.md delete mode 100644 .changeset/true-dolls-stay.md diff --git a/.changeset/eager-houses-call.md b/.changeset/eager-houses-call.md deleted file mode 100644 index 41929b61e..000000000 --- a/.changeset/eager-houses-call.md +++ /dev/null @@ -1,10 +0,0 @@ ---- -'@openfn/project': minor -'@openfn/cli': minor ---- - -Restructure `openfn.yaml` to make it safer for merging on github. - -Any checkout state (basically anything under the project key) have been moved into a new `.openfn` folder, which should not be tracked. - -`openfn.yaml` now just contains workspace-wide configuration. diff --git a/.changeset/hip-actors-switch.md b/.changeset/hip-actors-switch.md deleted file mode 100644 index 33e091794..000000000 --- a/.changeset/hip-actors-switch.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/cli': patch ---- - -Fix an issue where `project clean` can try to delete stuff even if nothing is checked out diff --git a/.changeset/orange-lions-talk.md b/.changeset/orange-lions-talk.md deleted file mode 100644 index 3a458c822..000000000 --- a/.changeset/orange-lions-talk.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/cli': patch ---- - -Include collections and channels in sync diffs diff --git a/.changeset/quiet-copies-match.md b/.changeset/quiet-copies-match.md deleted file mode 100644 index 87f270a4a..000000000 --- a/.changeset/quiet-copies-match.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -'@openfn/cli': patch -'@openfn/project': patch ---- - -Handle multiple local copies of the same project without reporting id conflicts diff --git a/.changeset/small-cases-buy.md b/.changeset/small-cases-buy.md deleted file mode 100644 index a05e27a3e..000000000 --- a/.changeset/small-cases-buy.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@openfn/cli': patch ---- - -When running commands with v2 sync, if no checkout metadata is detected, prompt the user diff --git a/.changeset/true-dolls-stay.md b/.changeset/true-dolls-stay.md deleted file mode 100644 index 3e537705b..000000000 --- a/.changeset/true-dolls-stay.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -'@openfn/project': patch -'@openfn/cli': patch ---- - -Track collections in resources.yaml diff --git a/integration-tests/cli/CHANGELOG.md b/integration-tests/cli/CHANGELOG.md index f35ddc411..7783b8536 100644 --- a/integration-tests/cli/CHANGELOG.md +++ b/integration-tests/cli/CHANGELOG.md @@ -1,5 +1,15 @@ # @openfn/integration-tests-cli +## 1.0.35 + +### Patch Changes + +- Updated dependencies [d3f1a1a] +- Updated dependencies [e90bfde] +- Updated dependencies [d3f1a1a] + - @openfn/project@0.22.0 + - @openfn/lightning-mock@2.5.5 + ## 1.0.34 ### Patch Changes diff --git a/integration-tests/cli/package.json b/integration-tests/cli/package.json index c4ea58a23..a3a76c1f7 100644 --- a/integration-tests/cli/package.json +++ b/integration-tests/cli/package.json @@ -1,7 +1,7 @@ { "name": "@openfn/integration-tests-cli", "private": true, - "version": "1.0.34", + "version": "1.0.35", "description": "CLI integration tests", "author": "Open Function Group ", "license": "ISC", diff --git a/packages/cli/CHANGELOG.md b/packages/cli/CHANGELOG.md index b431d9647..07936aada 100644 --- a/packages/cli/CHANGELOG.md +++ b/packages/cli/CHANGELOG.md @@ -1,5 +1,27 @@ # @openfn/cli +## 1.43.0 + +### Minor Changes + +- d3f1a1a: Restructure `openfn.yaml` to make it safer for merging on github. + + Any checkout state (basically anything under the project key) have been moved into a new `.openfn` folder, which should not be tracked. + + `openfn.yaml` now just contains workspace-wide configuration. + +### Patch Changes + +- d3f1a1a: Fix an issue where `project clean` can try to delete stuff even if nothing is checked out +- d3f1a1a: Include collections and channels in sync diffs +- e90bfde: Handle multiple local copies of the same project without reporting id conflicts +- d3f1a1a: When running commands with v2 sync, if no checkout metadata is detected, prompt the user +- d3f1a1a: Track collections in resources.yaml +- Updated dependencies [d3f1a1a] +- Updated dependencies [e90bfde] +- Updated dependencies [d3f1a1a] + - @openfn/project@0.22.0 + ## 1.42.0 ### Minor Changes diff --git a/packages/cli/package.json b/packages/cli/package.json index d834fcf17..15534f899 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/cli", - "version": "1.42.0", + "version": "1.43.0", "description": "CLI devtools for the OpenFn toolchain", "repository": { "url": "https://github.com/OpenFn/kit" diff --git a/packages/lightning-mock/CHANGELOG.md b/packages/lightning-mock/CHANGELOG.md index cd6d809cd..8e4c88dd9 100644 --- a/packages/lightning-mock/CHANGELOG.md +++ b/packages/lightning-mock/CHANGELOG.md @@ -1,5 +1,14 @@ # @openfn/lightning-mock +## 2.5.5 + +### Patch Changes + +- Updated dependencies [d3f1a1a] +- Updated dependencies [e90bfde] +- Updated dependencies [d3f1a1a] + - @openfn/project@0.22.0 + ## 2.5.4 ### Patch Changes diff --git a/packages/lightning-mock/package.json b/packages/lightning-mock/package.json index facaf3b79..56160488d 100644 --- a/packages/lightning-mock/package.json +++ b/packages/lightning-mock/package.json @@ -1,6 +1,6 @@ { "name": "@openfn/lightning-mock", - "version": "2.5.4", + "version": "2.5.5", "private": true, "description": "A mock Lightning server", "main": "dist/index.js", From fe7016ac6a51844ea9066f63ed434f782701b75a Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Fri, 9 Oct 2026 11:05:58 +0100 Subject: [PATCH 5/7] better guard and message on ad-hoc checkout --- packages/cli/src/projects/checkout.ts | 36 +++++- packages/cli/test/projects/checkout.test.ts | 135 ++++++++++++++++++++ 2 files changed, 169 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/projects/checkout.ts b/packages/cli/src/projects/checkout.ts index d004773cf..1cf57f400 100644 --- a/packages/cli/src/projects/checkout.ts +++ b/packages/cli/src/projects/checkout.ts @@ -26,6 +26,7 @@ export type CheckoutOptions = Pick< | 'workspace' | 'log' | 'clean' + | 'confirm' | 'force' | 'createCredentials' | 'branch' @@ -39,6 +40,7 @@ const options = [ po.track, po.clean, o.force, + o.confirm, po.creds, ]; @@ -138,12 +140,42 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { ); if (isAdHoc) { const trackedName = tracked!.alias ?? tracked!.id ?? tracked!.uuid; + + logger?.warn('Watch out! Ad-hoc checkout detected!'); + logger?.warn( + `You are on git branch ${options.branch}, which tracks project ${trackedName}` + ); logger?.warn( - `Branch ${options.branch} tracks project ${trackedName}. Expanding files from ${switchProject.alias} without changing tracking: deploy will still target ${trackedName}` + `But you've asked to checkout project ${switchProject.alias}.` ); logger?.warn( - `Pass --track to make ${options.branch} track ${switchProject.alias} instead` + `This will leave your branch (${trackedName}) inconsistent with your checked out workflows (${switchProject.alias}).` ); + logger?.warn( + `Pass --track to update your local tracker so that ${options.branch} tracks ${switchProject.alias} instead.` + ); + + // -f and -y both skip the prompt + const skip = options.force || options.confirm === false; + + // Without a terminal there's nobody to ask, and the prompt would hang + if (!skip && !process.stdin.isTTY) { + abort(logger!, 'Ad-hoc checkout needs to be confirmed', { + details: 'There is no terminal to ask for confirmation', + fix: `Pass --force (-f) or --confirm (-y) to continue anyway, or --track to make ${options.branch} track ${switchProject.alias}`, + }); + } + const doIt = logger + ? await logger.confirm( + `Continue and checkout ${switchProject.alias} anyway?`, + skip + ) + : true; + + if (!doIt) { + logger?.info('Checkout cancelled: nothing has been changed'); + return; + } } // delete workflow dir before expanding project diff --git a/packages/cli/test/projects/checkout.test.ts b/packages/cli/test/projects/checkout.test.ts index 7c190e305..b2260ff43 100644 --- a/packages/cli/test/projects/checkout.test.ts +++ b/packages/cli/test/projects/checkout.test.ts @@ -1057,3 +1057,138 @@ test.serial( ); } ); + +// Ad-hoc checkouts: checking out a project on a git branch that tracks a +// different project. dev tracks staging, and we try to checkout the main project +const adHocSetup = () => { + fs.mkdirSync('/ws/.openfn/branches/dev', { recursive: true }); + fs.writeFileSync( + '/ws/.openfn/branches/dev/checkout.yaml', + 'id: staging\nuuid: \n' + ); +}; + +// The prompt needs a terminal (without one, ad-hoc checkouts abort) +const withTTY = async (fn: () => Promise) => { + const stdin: any = process.stdin; + const original = stdin.isTTY; + stdin.isTTY = true; + try { + await fn(); + } finally { + stdin.isTTY = original; + } +}; + +const adHocCheckout = (log: any, extra = {}) => + checkoutHandler( + { + command: 'project-checkout', + project: 'project', + workspace: '/ws', + branch: 'dev', + ...extra, + }, + log + ); + +// The real logger skips the prompt if confirm() is called with force=true, +// but the mock always records and approves, so spy on the arguments instead +const spyLogger = (answer = true) => { + const calls: Array<{ message: string; skip?: boolean }> = []; + const log: any = { + ...createMockLogger(undefined, { level: 'debug' }), + confirm: async (message: string, skip?: boolean) => { + calls.push({ message, skip }); + return answer; + }, + }; + return { log, calls }; +}; + +test.serial( + 'checkout: asks for confirmation on an ad-hoc checkout', + async (t) => { + adHocSetup(); + const { log, calls } = spyLogger(); + + await withTTY(() => adHocCheckout(log)); + + t.is(calls.length, 1); + t.regex(calls[0].message, /Continue and checkout .* anyway/); + t.falsy(calls[0].skip); + // the files were expanded + t.true(fs.existsSync('/ws/workflows/simple-workflow-main')); + // but the branch still tracks staging + t.is( + fs.readFileSync('/ws/.openfn/branches/dev/checkout.yaml', 'utf8'), + 'id: staging\nuuid: \n' + ); + } +); + +test.serial( + 'checkout: does nothing if the user declines an ad-hoc checkout', + async (t) => { + adHocSetup(); + const { log } = spyLogger(false); + + await withTTY(() => adHocCheckout(log)); + + t.false(fs.existsSync('/ws/workflows/simple-workflow-main')); + } +); + +test.serial('checkout: -f skips the ad-hoc confirmation', async (t) => { + adHocSetup(); + const { log, calls } = spyLogger(); + + await adHocCheckout(log, { force: true }); + + t.true(calls[0].skip); + t.true(fs.existsSync('/ws/workflows/simple-workflow-main')); +}); + +test.serial('checkout: -y skips the ad-hoc confirmation', async (t) => { + adHocSetup(); + const { log, calls } = spyLogger(); + + await adHocCheckout(log, { confirm: false }); + + t.true(calls[0].skip); + t.true(fs.existsSync('/ws/workflows/simple-workflow-main')); +}); + +test.serial( + 'checkout: does not ask for confirmation if the branch is not tracking something else', + async (t) => { + fs.mkdirSync('/ws/.openfn/branches/dev', { recursive: true }); + fs.writeFileSync( + '/ws/.openfn/branches/dev/checkout.yaml', + 'id: main\nuuid: \n' + ); + const { log, calls } = spyLogger(); + + await adHocCheckout(log); + + t.is(calls.length, 0); + } +); + +test.serial( + 'checkout: aborts an ad-hoc checkout with no terminal and no -f or -y', + async (t) => { + adHocSetup(); + const { log, calls } = spyLogger(); + const exitCode = process.exitCode; + + await t.throwsAsync(adHocCheckout(log), { + message: 'Ad-hoc checkout needs to be confirmed', + }); + // abort() flags the process as failed: don't let that leak out of the test + process.exitCode = exitCode; + + t.is(calls.length, 0); + t.false(fs.existsSync('/ws/workflows/simple-workflow-main')); + } +); From cbe91d7c92a65c58bcb880e35128593ea190ef70 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 12 Oct 2026 09:56:56 +0100 Subject: [PATCH 6/7] revert ensureCheckoutIgnored We need to track the checkout files in github for github sync deploy actions to work properly (without the checkout files, the sync aciton cannot connect workflows/ to a deploy target) --- packages/cli/src/projects/checkout.ts | 7 +--- packages/cli/src/projects/util.ts | 16 -------- packages/cli/test/projects/util.test.ts | 50 ------------------------- 3 files changed, 1 insertion(+), 72 deletions(-) diff --git a/packages/cli/src/projects/checkout.ts b/packages/cli/src/projects/checkout.ts index 1cf57f400..9a9dd4973 100644 --- a/packages/cli/src/projects/checkout.ts +++ b/packages/cli/src/projects/checkout.ts @@ -11,11 +11,7 @@ import * as o from '../options'; import * as po from './options'; import type { Opts } from './options'; -import { - ensureCheckoutIgnored, - tidyWorkflowDir, - updateForkedFrom, -} from './util'; +import { tidyWorkflowDir, updateForkedFrom } from './util'; import { createProjectCredentials } from './credentials-helpers'; import abort from '../util/abort'; @@ -203,7 +199,6 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { delete files[switchProject.generateConfig().path]; delete files[switchProject.generateCheckout(options.branch).path]; } - await ensureCheckoutIgnored(workspacePath); for (const f in files) { if (files[f]) { fs.mkdirSync(path.join(workspacePath, path.dirname(f)), { diff --git a/packages/cli/src/projects/util.ts b/packages/cli/src/projects/util.ts index b77d7dd97..a34e8d7d2 100644 --- a/packages/cli/src/projects/util.ts +++ b/packages/cli/src/projects/util.ts @@ -260,21 +260,6 @@ export const updateForkedFrom = (proj: Project) => { return proj; }; -// The checkout state in .openfn is local to each machine, so it shouldn't be -// committed. Make git ignore everything in there. -// Set OPENFN_IGNORE_CHECKOUT_META=true to skip this -export const ensureCheckoutIgnored = async (workspacePath: string) => { - if (process.env.OPENFN_IGNORE_CHECKOUT_META === 'true') { - return; - } - - const filePath = path.resolve(workspacePath, '.openfn', '.gitignore'); - if (!fs.existsSync(filePath)) { - await mkdir(path.dirname(filePath), { recursive: true }); - await writeFile(filePath, '*\n'); - } -}; - // Set up the checkout file for a project without touching anything else // (use this when the workflows are already on disk, like after a git clone) export const writeCheckoutFile = async ( @@ -283,7 +268,6 @@ export const writeCheckoutFile = async ( branch?: string | false | null ) => { updateForkedFrom(project); - await ensureCheckoutIgnored(workspacePath); const { path: checkoutPath, content } = project.generateCheckout(branch); const filePath = path.resolve(workspacePath, checkoutPath); diff --git a/packages/cli/test/projects/util.test.ts b/packages/cli/test/projects/util.test.ts index 453e92d31..d9679494f 100644 --- a/packages/cli/test/projects/util.test.ts +++ b/packages/cli/test/projects/util.test.ts @@ -6,7 +6,6 @@ import { findLocallyChangedWorkflows, tidyWorkflowDir, writeCheckoutFile, - ensureCheckoutIgnored, } from '../../src/projects/util'; test.afterEach(() => { @@ -342,52 +341,3 @@ test.serial( t.deepEqual(fs.readdirSync('/ws/workflows/wf'), ['wf.yaml']); } ); - -test.serial( - 'ensureCheckoutIgnored: makes git ignore everything in .openfn', - async (t) => { - mock({ '/ws': {} }); - - await ensureCheckoutIgnored('/ws'); - - t.is(fs.readFileSync('/ws/.openfn/.gitignore', 'utf8'), '*\n'); - } -); - -test.serial( - 'ensureCheckoutIgnored: leaves an existing .gitignore alone', - async (t) => { - mock({ '/ws/.openfn/.gitignore': 'custom\n' }); - - await ensureCheckoutIgnored('/ws'); - - t.is(fs.readFileSync('/ws/.openfn/.gitignore', 'utf8'), 'custom\n'); - } -); - -test.serial( - 'ensureCheckoutIgnored: does nothing if OPENFN_IGNORE_CHECKOUT_META is set', - async (t) => { - mock({ '/ws': {} }); - process.env.OPENFN_IGNORE_CHECKOUT_META = 'true'; - - try { - await ensureCheckoutIgnored('/ws'); - } finally { - delete process.env.OPENFN_IGNORE_CHECKOUT_META; - } - - t.false(fs.existsSync('/ws/.openfn')); - } -); - -test.serial( - 'writeCheckoutFile: makes git ignore the checkout state', - async (t) => { - mock({ '/ws': {} }); - - await writeCheckoutFile('/ws', trackedProject()); - - t.is(fs.readFileSync('/ws/.openfn/.gitignore', 'utf8'), '*\n'); - } -); From 9f844aceaddee637313d37353c68d119494e3969 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Mon, 12 Oct 2026 10:22:47 +0100 Subject: [PATCH 7/7] unit test --- packages/cli/test/projects/checkout.test.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/cli/test/projects/checkout.test.ts b/packages/cli/test/projects/checkout.test.ts index b2260ff43..6f6827356 100644 --- a/packages/cli/test/projects/checkout.test.ts +++ b/packages/cli/test/projects/checkout.test.ts @@ -222,15 +222,6 @@ test.serial('checkout: same id as active', async (t) => { ); }); -test.serial('checkout: makes git ignore the checkout state', async (t) => { - await checkoutHandler( - { command: 'project-checkout', project: 'my-project', workspace: '/ws' }, - logger - ); - - t.is(fs.readFileSync('/ws/.openfn/.gitignore', 'utf8'), '*\n'); -}); - test.serial( 'checkout: writes forked_from based on version history', async (t) => {