diff --git a/.changeset/olive-rats-kick.md b/.changeset/olive-rats-kick.md new file mode 100644 index 000000000..30ab43f81 --- /dev/null +++ b/.changeset/olive-rats-kick.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': minor +--- + +Remove renamed/deleted workflows or steps during checkout diff --git a/.claude/command-refactor.md b/.claude/command-refactor.md index ea6930782..9359ee505 100644 --- a/.claude/command-refactor.md +++ b/.claude/command-refactor.md @@ -13,6 +13,7 @@ Use `src/projects/list.ts` as the reference pattern for all refactored commands. Create a new file in `src/projects/.ts` that consolidates all the existing command files. **Key elements:** + - Import `yargs`, `ensure`, `build`, `Logger`, and options from `../options` - Define a `Options` type that picks required fields from `Opts` - Create an `options` array with the required options (e.g., `[o.workflow, o.workspace, o.workflowMappings]`) @@ -24,6 +25,7 @@ Create a new file in `src/projects/.ts` that consolidates all the - Export a named `handler` function that takes `(options: Options, logger: Logger)` **Example structure:** + ```typescript import yargs from 'yargs'; import { Workspace } from '@openfn/project'; @@ -88,23 +90,26 @@ export const projectsCommand = { Make three changes: **a) Remove the old import** (if it exists): + ```typescript // Remove: import commandName from './/handler'; ``` **b) Add to CommandList type:** + ```typescript export type CommandList = | 'apollo' // ... | 'project-list' | 'project-version' - | 'project-' // Add this line + | 'project-' // Add this line | 'test' | 'version'; ``` **c) Add to handlers object:** + ```typescript const handlers = { // ... @@ -121,6 +126,7 @@ If the old command was referenced elsewhere in the handlers object (like `projec ### 5. Delete the Old Command Folder Remove the old command directory: + ```bash rm -rf packages/cli/src/ ``` @@ -142,6 +148,7 @@ import checkoutCommand from './checkout/command'; ``` The command will be registered with yargs: + ```typescript .command(projectsCommand) .command(mergeCommand) // Top-level alias @@ -159,18 +166,21 @@ This is different from the `install`/`repo install` pattern, where both entries ## Common Patterns ### Options to Use + - Always include `o.workspace` for project-related commands - Use `o.workflow` for workflow-specific operations - Include `o.json` if the command supports JSON output - Include `o.log` for commands that need detailed logging ### Handler Pattern + - Use `new Workspace(options.workspace)` to access the workspace - Check `workspace.valid` before proceeding -- Use `workspace.getActiveProject()` to get the current project +- Use `workspace.getTrackedProject()` to get the current project - Use appropriate logger methods: `logger.info()`, `logger.error()`, `logger.success()` ### Testing Pattern + If the old command has tests, they need to be refactored: 1. **Create new test file**: Move from `test//handler.test.ts` to `test/projects/.test.ts` @@ -181,6 +191,7 @@ If the old command has tests, they need to be refactored: 4. **Delete old test folder**: Remove `test/` directory **Example changes:** + ```typescript // Before: import mergeHandler from '../../src/merge/handler'; @@ -194,6 +205,7 @@ await mergeHandler({ command: 'project-merge', ... }, logger); ## Checklist ### Basic Refactoring + - [ ] Create new file in `src/projects/.ts` - [ ] Define `Options` type - [ ] Export default command object with `ensure('project-', options)` @@ -206,6 +218,7 @@ await mergeHandler({ command: 'project-merge', ... }, logger); - [ ] Delete old command folder ### Testing (if applicable) + - [ ] Create new test file in `test/projects/.test.ts` - [ ] Update import to use `{ handler } from '../../src/projects/'` - [ ] Update all `command: ''` to `command: 'project-'` in test cases @@ -213,6 +226,7 @@ await mergeHandler({ command: 'project-merge', ... }, logger); - [ ] Run tests to verify they pass ### Additional Steps for Top-Level Aliasing + - [ ] Import command directly in `src/cli.ts` (e.g., `import mergeCommand from './projects/merge'`) - [ ] Register the imported command with `.command(mergeCommand)` - [ ] Verify NO duplicate entries in `src/commands.ts` - only `project-` should exist, not `` diff --git a/packages/cli/src/projects/checkout.ts b/packages/cli/src/projects/checkout.ts index 5ae52a2f7..c81351a85 100644 --- a/packages/cli/src/projects/checkout.ts +++ b/packages/cli/src/projects/checkout.ts @@ -46,8 +46,7 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { // TODO: try to retain the endpoint for the projects const { project: _, ...config } = workspace.getConfig() as any; - const currentProject = workspace.getActiveProject(); - + const currentProject = await workspace.getCheckedOutProject(); // get the project let switchProject; if (/\.(yaml|json)$/.test(projectIdentifier)) { @@ -108,7 +107,7 @@ export const handler = async (options: CheckoutOptions, logger?: Logger) => { if (options.clean) { await rimraf(workspace.workflowsPath); } else { - await tidyWorkflowDir(currentProject!, switchProject); + await tidyWorkflowDir(currentProject, switchProject, false, workspacePath); } // write the forked from map diff --git a/packages/cli/src/projects/deploy.ts b/packages/cli/src/projects/deploy.ts index 8549d481e..c355f2773 100644 --- a/packages/cli/src/projects/deploy.ts +++ b/packages/cli/src/projects/deploy.ts @@ -251,7 +251,7 @@ export async function handler(options: DeployOptions, logger: Logger) { // We need track alias in openfn.yaml to make this easier (and tracked in from fs) const ws = new Workspace(options.workspace || '.'); - const active = ws.getActiveProject(); + const active = ws.getTrackedProject(); const alias = options.alias ?? active?.alias; const localProject = await Project.from('fs', { diff --git a/packages/cli/src/projects/list.ts b/packages/cli/src/projects/list.ts index dc1e84848..06d091c59 100644 --- a/packages/cli/src/projects/list.ts +++ b/packages/cli/src/projects/list.ts @@ -39,7 +39,7 @@ export const handler = async (options: ProjectListOptions, logger: Logger) => { logger.always(`Available openfn projects\n\n${workspace .list() - .map((p) => describeProject(p, p === workspace.getActiveProject())) + .map((p) => describeProject(p, p === workspace.getTrackedProject())) .join('\n\n')} `); }; diff --git a/packages/cli/src/projects/merge.ts b/packages/cli/src/projects/merge.ts index 6b471e39a..7c9837c7d 100644 --- a/packages/cli/src/projects/merge.ts +++ b/packages/cli/src/projects/merge.ts @@ -71,7 +71,7 @@ export const handler = async (options: MergeOptions, logger: Logger) => { logger.debug('Loading target project from path', basePath); targetProject = await Project.from('path', basePath); } else { - targetProject = workspace.getActiveProject()!; + targetProject = workspace.getTrackedProject()!; if (!targetProject) { logger.error(`No project currently checked out`); return; diff --git a/packages/cli/src/projects/util.ts b/packages/cli/src/projects/util.ts index 43f847add..7a9185585 100644 --- a/packages/cli/src/projects/util.ts +++ b/packages/cli/src/projects/util.ts @@ -1,4 +1,5 @@ import path from 'node:path'; +import fs from 'node:fs'; import { mkdir, writeFile } from 'node:fs/promises'; import { Provisioner } from '@openfn/lexicon/lightning'; import { fetch } from 'undici'; @@ -214,7 +215,8 @@ class DeployError extends Error { export async function tidyWorkflowDir( currentProject: Project | undefined, incomingProject: Project | undefined, - dryRun = false + dryRun = false, + dirPath = '.' ) { if (!currentProject || !incomingProject) { return []; @@ -232,7 +234,15 @@ export async function tidyWorkflowDir( } if (!dryRun) { - await rimraf(toRemove); + const abs = (p: string) => path.join(dirPath, p); + await rimraf(toRemove.map(abs)); + + const dirs = new Set(toRemove.map((f) => path.dirname(f))); + for (const dir of Array.from(dirs)) { + try { + fs.rmdirSync(abs(dir)); // throws when not empty + } catch {} + } } // Return and sort for testing diff --git a/packages/cli/src/projects/version.ts b/packages/cli/src/projects/version.ts index dfe0c90bf..94b20a893 100644 --- a/packages/cli/src/projects/version.ts +++ b/packages/cli/src/projects/version.ts @@ -32,7 +32,7 @@ export const handler = async (options: VersionOptions, logger: Logger) => { const output = new Map(); - const activeProject = workspace.getActiveProject(); + const activeProject = await workspace.getCheckedOutProject(); if (options.workflow) { const workflow = activeProject?.getWorkflow(options.workflow); if (!workflow) { diff --git a/packages/cli/src/util/load-plan.ts b/packages/cli/src/util/load-plan.ts index 203a7acb9..67180457d 100644 --- a/packages/cli/src/util/load-plan.ts +++ b/packages/cli/src/util/load-plan.ts @@ -62,7 +62,7 @@ const loadPlan = async ( }; options.credentials ??= workspace.getConfig().credentials; - options.collectionsEndpoint ??= proj.openfn?.endpoint; + options.collectionsEndpoint ??= proj?.openfn?.endpoint; // Set the cache path to be relative to the workflow options.cachePath ??= workspace.workflowsPath + `/${name}/${CACHE_DIR}`; } diff --git a/packages/cli/test/projects/checkout.test.ts b/packages/cli/test/projects/checkout.test.ts index 8f7f74add..2208de789 100644 --- a/packages/cli/test/projects/checkout.test.ts +++ b/packages/cli/test/projects/checkout.test.ts @@ -499,3 +499,127 @@ workspace: ], }); }); + +test.serial( + 'checkout: removes old workflow directory when workflow is renamed on server', + async (t) => { + mock({ + '/ws3/workflows/old-workflow': { + 'old-workflow.yaml': jsonToYaml({ + id: 'old-workflow', + steps: [{ id: 'run-job', expression: './run-job.js' }], + }), + 'run-job.js': 'fn(state => state)', + }, + '/ws3/openfn.yaml': jsonToYaml({ project: { id: 'main-project' } }), + '/ws3/.projects/main-project@server.yaml': jsonToYaml({ + id: '', + name: 'Main Project', + workflows: [ + { + name: 'New Workflow', + jobs: [{ name: 'Run Job', body: 'fn(s => s)' }], + triggers: [], + edges: [], + }, + ], + }), + }); + + await checkoutHandler( + { + command: 'project-checkout', + project: 'main-project', + workspace: '/ws3', + }, + logger + ); + + t.false(fs.existsSync('/ws3/workflows/old-workflow')); + t.true(fs.existsSync('/ws3/workflows/new-workflow')); + } +); + +test.serial( + 'checkout: removes old step file when step is renamed on server', + async (t) => { + mock({ + '/ws4/workflows/my-workflow': { + 'my-workflow.yaml': jsonToYaml({ + id: 'my-workflow', + steps: [{ id: 'old-step', expression: './old-step.js' }], + }), + 'old-step.js': 'fn(state => state)', + }, + '/ws4/openfn.yaml': jsonToYaml({ project: { id: 'main-project' } }), + '/ws4/.projects/main-project@server.yaml': jsonToYaml({ + id: '', + name: 'Main Project', + workflows: [ + { + name: 'My Workflow', + jobs: [{ name: 'New Step', body: 'fn(s => s)' }], + triggers: [], + edges: [], + }, + ], + }), + }); + + await checkoutHandler( + { + command: 'project-checkout', + project: 'main-project', + workspace: '/ws4', + }, + logger + ); + + t.false(fs.existsSync('/ws4/workflows/my-workflow/old-step.js')); + t.true(fs.existsSync('/ws4/workflows/my-workflow/new-step.js')); + t.true(fs.existsSync('/ws4/workflows/my-workflow')); + } +); + +test.serial( + 'checkout: removes workflow directory when workflow is deleted on server', + async (t) => { + mock({ + '/ws5/workflows/workflow-a': { + 'workflow-a.yaml': jsonToYaml({ + id: 'workflow-a', + steps: [{ id: 'run-job', expression: './run-job.js' }], + }), + 'run-job.js': 'fn(state => state)', + }, + '/ws5/workflows/workflow-b': { + 'workflow-b.yaml': jsonToYaml({ id: 'workflow-b', steps: [] }), + }, + '/ws5/openfn.yaml': jsonToYaml({ project: { id: 'main-project' } }), + '/ws5/.projects/main-project@server.yaml': jsonToYaml({ + id: '', + name: 'Main Project', + workflows: [ + { + name: 'Workflow A', + jobs: [{ name: 'Run Job', body: 'fn(s => s)' }], + triggers: [], + edges: [], + }, + ], + }), + }); + + await checkoutHandler( + { + command: 'project-checkout', + project: 'main-project', + workspace: '/ws5', + }, + logger + ); + + t.false(fs.existsSync('/ws5/workflows/workflow-b')); + t.true(fs.existsSync('/ws5/workflows/workflow-a')); + } +); diff --git a/packages/project/src/Workspace.ts b/packages/project/src/Workspace.ts index 618bdbcf2..0f7eb847b 100644 --- a/packages/project/src/Workspace.ts +++ b/packages/project/src/Workspace.ts @@ -116,15 +116,21 @@ export class Workspace { return this.projectPaths.get(id); } - getActiveProject() { + getTrackedProject() { return ( this.projects.find((p) => p.openfn?.uuid === this.activeProject?.uuid) ?? this.projects.find((p) => p.id === this.activeProject?.id) ); } - getCheckedOutProject() { - return Project.from('fs', { root: this.root, config: this.config }); + async getCheckedOutProject() { + return await Project.from('fs', { + root: this.root, + config: this.config, + }).catch((e) => { + if (e.code === 'ENOENT') return undefined; + throw e; + }); } getCredentialMap() {