From c84f75bb21ac50b6555193df036ccbe47c221fd3 Mon Sep 17 00:00:00 2001 From: Lamine Gueye <51838776+lamine2000@users.noreply.github.com> Date: Tue, 9 Jun 2026 15:17:35 +0000 Subject: [PATCH 1/5] fix(cli): fix cache .gitignore written to fs root and undefined workflow name (#1444) * fix(cli): fix cache .gitignore written to fs root and undefined workflow name - Replace ensureGitIgnore path-walking logic with a direct getCacheRoot helper. The old loop used endsWith('.cli-cache') to find the cache root, but if the path never contained that segment (custom cachePath) it would walk all the way to '/' and write /.gitignore - Fall back to 'workflow' when plan.workflow.name is undefined, avoiding .cli-cache/undefined directories - Add cachePath to saveToCache/clearCache options types so custom cache paths set by workspace projects are correctly forwarded to getCachePath - Remove spurious await on the now-synchronous getCachePath call - Add regression tests covering expressionPath + cacheSteps (the exact scenario reported in #669, which had no test coverage) Fixes #669 * fix test: use correct cache subdir name derived from filename * refactor(cli): address review feedback on cache fix - Eliminate getCacheRoot helper: getCachePath(options) with no workflowName/stepId already returns the CACHE_DIR root naturally - Use .filter(Boolean) spread into path.resolve so undefined workflowName does not cause ERR_INVALID_ARG_TYPE (path.resolve throws on undefined) - Keep cachePath in saveToCache/clearCache Pick types so custom paths work - Rename gitignore test name to be more accurate Co-Authored-By: Claude Sonnet 4.6 * style(cli): fix prettier formatting Co-Authored-By: Claude Sonnet 4.6 * Revert "style(cli): fix prettier formatting" This reverts commit 1e47fad4e70123353c2ab77bed34cb4fa51bc154. * style(cli): fix prettier v2 formatting Co-Authored-By: Claude Sonnet 4.6 * refactor(cli): simplify workflowName fallback per review Use workflowName ?? '' instead of .filter(Boolean) spread. path.resolve treats '' as a no-op so undefined workflowName still resolves to the bare CACHE_DIR root. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- packages/cli/src/util/cache.ts | 29 ++++++-------- packages/cli/test/execute/execute.test.ts | 49 +++++++++++++++++++++++ 2 files changed, 62 insertions(+), 16 deletions(-) diff --git a/packages/cli/src/util/cache.ts b/packages/cli/src/util/cache.ts index 846cb9cf2..6113509f8 100644 --- a/packages/cli/src/util/cache.ts +++ b/packages/cli/src/util/cache.ts @@ -8,7 +8,8 @@ import type { Logger } from './logger'; export const CACHE_DIR = '.cli-cache'; -// TODO this is all a bit over complicated tbh +// When called without workflowName/stepId, returns the CACHE_DIR root. +// This is used directly in saveToCache to locate the .gitignore. export const getCachePath = ( options: Pick, workflowName?: string, @@ -24,7 +25,8 @@ export const getCachePath = ( const basePath = path.resolve( baseDir ?? process.cwd(), - `${CACHE_DIR}/${workflowName}` + CACHE_DIR, + workflowName ?? '' ); if (stepId) { @@ -33,38 +35,33 @@ export const getCachePath = ( return basePath; }; -const ensureGitIgnore = (options: any, cachePath: string) => { +const ensureGitIgnore = (options: any, cacheRoot: string) => { if (!options._hasGitIgnore) { - // Find the root cache folder - let root = cachePath; - while (root.length > 1 && !root.endsWith(CACHE_DIR)) { - root = path.dirname(root); - } - // From the root cache, look for a .gitignore - const ignorePath = path.resolve(root, '.gitignore'); + const ignorePath = path.join(cacheRoot, '.gitignore'); try { fs.accessSync(ignorePath); } catch (e) { // doesn't exist! fs.writeFileSync(ignorePath, '*'); } + options._hasGitIgnore = true; } - options._hasGitIgnore = true; }; export const saveToCache = async ( plan: ExecutionPlan, stepId: string, output: any, - options: Pick, + options: Pick, logger: Logger ) => { if (options.cacheSteps) { - const cachePath = await getCachePath(options, plan.workflow.name, stepId); + const cachePath = getCachePath(options, plan.workflow.name, stepId); // Note that this is sync because other execution order gets messed up fs.mkdirSync(path.dirname(cachePath), { recursive: true }); - ensureGitIgnore(options, path.dirname(cachePath)); + // getCachePath with no workflowName returns the CACHE_DIR root + ensureGitIgnore(options, getCachePath(options)); logger.info(`Writing ${stepId} output to ${cachePath}`); fs.writeFileSync(cachePath, JSON.stringify(output)); @@ -73,10 +70,10 @@ export const saveToCache = async ( export const clearCache = async ( plan: ExecutionPlan, - options: Pick, + options: Pick, logger: Logger ) => { - const cacheDir = await getCachePath(options, plan.workflow?.name); + const cacheDir = getCachePath(options, plan.workflow?.name); try { await rmdir(cacheDir, { recursive: true }); diff --git a/packages/cli/test/execute/execute.test.ts b/packages/cli/test/execute/execute.test.ts index 2b500fa1b..f96ee0c32 100644 --- a/packages/cli/test/execute/execute.test.ts +++ b/packages/cli/test/execute/execute.test.ts @@ -418,6 +418,55 @@ test.serial('.cli-cache has a gitignore', async (t) => { t.is(gitignore, '*'); }); +// Regression test for https://github.com/OpenFn/kit/issues/669 +test.serial('cache steps when running a .js expression', async (t) => { + mockFs({ + '/job.js': `${fn}fn((state) => ({ ...state, x: 1 }));`, + }); + + const options = { + ...defaultOptions, + expressionPath: '/job.js', + baseDir: '/', + cacheSteps: true, + }; + + const result = await handler(options, logger); + t.is(result.x, 1); + + // workflow name is derived from the filename: 'job' + // step id is a generated uuid, so list the directory + const files = await fs.readdir('/.cli-cache/job'); + t.is(files.length, 1); + t.true(files[0].endsWith('.json')); + + const cached = JSON.parse( + await fs.readFile(`/.cli-cache/job/${files[0]}`, 'utf8') + ); + t.is(cached.x, 1); +}); + +test.serial( + '.cli-cache has a gitignore when caching a .js expression', + async (t) => { + mockFs({ + '/job.js': `${fn}fn((state) => ({ ...state, x: 1 }));`, + }); + + const options = { + ...defaultOptions, + expressionPath: '/job.js', + baseDir: '/', + cacheSteps: true, + }; + + await handler(options, logger); + + const gitignore = await fs.readFile('/.cli-cache/.gitignore', 'utf8'); + t.is(gitignore, '*'); + } +); + test.serial('run a workflow with initial state from stdin', async (t) => { const workflow = { workflow: { From 36dd72d3105fe3caef1640b9034af17678ec258b Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 9 Jun 2026 16:27:05 +0100 Subject: [PATCH 2/5] clean up test --- packages/cli/test/execute/execute.test.ts | 41 +++-------------------- 1 file changed, 5 insertions(+), 36 deletions(-) diff --git a/packages/cli/test/execute/execute.test.ts b/packages/cli/test/execute/execute.test.ts index f96ee0c32..8b7184aff 100644 --- a/packages/cli/test/execute/execute.test.ts +++ b/packages/cli/test/execute/execute.test.ts @@ -418,8 +418,7 @@ test.serial('.cli-cache has a gitignore', async (t) => { t.is(gitignore, '*'); }); -// Regression test for https://github.com/OpenFn/kit/issues/669 -test.serial('cache steps when running a .js expression', async (t) => { +test.serial('.cli-cache writes a .gitignore', async (t) => { mockFs({ '/job.js': `${fn}fn((state) => ({ ...state, x: 1 }));`, }); @@ -431,41 +430,11 @@ test.serial('cache steps when running a .js expression', async (t) => { cacheSteps: true, }; - const result = await handler(options, logger); - t.is(result.x, 1); - - // workflow name is derived from the filename: 'job' - // step id is a generated uuid, so list the directory - const files = await fs.readdir('/.cli-cache/job'); - t.is(files.length, 1); - t.true(files[0].endsWith('.json')); - - const cached = JSON.parse( - await fs.readFile(`/.cli-cache/job/${files[0]}`, 'utf8') - ); - t.is(cached.x, 1); -}); - -test.serial( - '.cli-cache has a gitignore when caching a .js expression', - async (t) => { - mockFs({ - '/job.js': `${fn}fn((state) => ({ ...state, x: 1 }));`, - }); - - const options = { - ...defaultOptions, - expressionPath: '/job.js', - baseDir: '/', - cacheSteps: true, - }; - - await handler(options, logger); + await handler(options, logger); - const gitignore = await fs.readFile('/.cli-cache/.gitignore', 'utf8'); - t.is(gitignore, '*'); - } -); + const gitignore = await fs.readFile('/.cli-cache/.gitignore', 'utf8'); + t.is(gitignore, '*'); +}); test.serial('run a workflow with initial state from stdin', async (t) => { const workflow = { From e68c9b288ab99acb8d9d7f3ce933cf8d35524a92 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 9 Jun 2026 16:28:31 +0100 Subject: [PATCH 3/5] changeset --- .changeset/chubby-lines-think.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/chubby-lines-think.md diff --git a/.changeset/chubby-lines-think.md b/.changeset/chubby-lines-think.md new file mode 100644 index 000000000..e7e434f9b --- /dev/null +++ b/.changeset/chubby-lines-think.md @@ -0,0 +1,5 @@ +--- +'@openfn/cli': patch +--- + +Write the .gitignore file for cli cache to the cache root. This means the command will generate more ignore files, but prevents them getting generated at the system root From 720ffd1d844fb71756d2f2d3082755018d36e14c Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 9 Jun 2026 17:07:03 +0100 Subject: [PATCH 4/5] tidying up --- packages/cli/src/execute/handler.ts | 9 --------- packages/cli/src/util/cache.ts | 6 +++--- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/execute/handler.ts b/packages/cli/src/execute/handler.ts index 7c926102c..de687a2c7 100644 --- a/packages/cli/src/execute/handler.ts +++ b/packages/cli/src/execute/handler.ts @@ -175,15 +175,6 @@ const executeHandler = async (options: ExecuteOptions, logger: Logger) => { try { const result = await execute(finalPlan, state, options, logger); - if (options.cacheSteps) { - logger.success( - `Cached output written to ${getCachePath( - options, - plan.workflow.name - )} (see info logs for details)` - ); - } - await serializeOutput(options, result, logger); const duration = printDuration(new Date().getTime() - start); if (result?.errors) { diff --git a/packages/cli/src/util/cache.ts b/packages/cli/src/util/cache.ts index 6113509f8..0f4dbf1a0 100644 --- a/packages/cli/src/util/cache.ts +++ b/packages/cli/src/util/cache.ts @@ -35,13 +35,13 @@ export const getCachePath = ( return basePath; }; -const ensureGitIgnore = (options: any, cacheRoot: string) => { +const ensureGitIgnore = (options: any, cacheRoot: string, logger?: Logger) => { if (!options._hasGitIgnore) { const ignorePath = path.join(cacheRoot, '.gitignore'); try { fs.accessSync(ignorePath); } catch (e) { - // doesn't exist! + logger?.debug('Creating .gitignore at ', ignorePath); fs.writeFileSync(ignorePath, '*'); } options._hasGitIgnore = true; @@ -63,7 +63,7 @@ export const saveToCache = async ( // getCachePath with no workflowName returns the CACHE_DIR root ensureGitIgnore(options, getCachePath(options)); - logger.info(`Writing ${stepId} output to ${cachePath}`); + logger.info(`Writing cached ${stepId} output to ${cachePath}`); fs.writeFileSync(cachePath, JSON.stringify(output)); } }; From 105132500c0cc547a5d1278f87ffd7e598b93c80 Mon Sep 17 00:00:00 2001 From: Joe Clark Date: Tue, 9 Jun 2026 17:12:38 +0100 Subject: [PATCH 5/5] typing --- packages/cli/src/execute/handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli/src/execute/handler.ts b/packages/cli/src/execute/handler.ts index de687a2c7..607e73b33 100644 --- a/packages/cli/src/execute/handler.ts +++ b/packages/cli/src/execute/handler.ts @@ -15,7 +15,7 @@ import loadState from '../util/load-state'; import validateAdaptors from '../util/validate-adaptors'; import loadPlan from '../util/load-plan'; import assertPath from '../util/assert-path'; -import { clearCache, getCachePath } from '../util/cache'; +import { clearCache } from '../util/cache'; import fuzzyMatchStep from '../util/fuzzy-match-step'; import abort from '../util/abort'; import validatePlan from '../util/validate-plan';