fix(cli): fix cache .gitignore written to fs root and undefined workflow name

[lamine2000]

Summary

Fixes the CLI cache bug reported in #669 where running a .js expression with caching enabled (via OPENFN_ALWAYS_CACHE_STEPS=true or --cache-steps) caused a TypeError [ERR_INVALID_ARG_TYPE] crash.

• ensureGitIgnore rewritten: The old implementation walked up the cache file path looking for a segment ending in .cli-cache. If the path did not contain that segment (e.g. a custom cachePath), the loop traversed all the way to the filesystem root and wrote /.gitignore. Replaced with a direct call to getCachePath(options) (no workflowName/stepId), which naturally returns the cache root, so no traversal is needed.
• Undefined workflow name crash fixed: path.resolve throws ERR_INVALID_ARG_TYPE when passed undefined. getCachePath now uses .filter(Boolean) before spreading into path.resolve, so an absent plan.workflow.name is simply omitted from the path rather than crashing.
• Eliminated getCacheRoot helper: After the above two fixes, getCachePath(options) with no extra arguments already returns the cache root directly — no separate helper needed.
• Type fix for saveToCache and clearCache: Both were typed as Pick<Opts, 'baseDir' | 'cacheSteps'>, missing cachePath, so the cachePath branch inside getCachePath was unreachable from these functions. Added cachePath to both Pick types.

Test plan

• Added cache steps when running a .js expression — runs an expression with cacheSteps: true and verifies the cache file is written under .cli-cache/<name>/ (this is the exact scenario from CLI cache bug #669, which had zero test coverage)
• Added .cli-cache has a gitignore when caching a .js expression — verifies .gitignore is correctly placed in .cli-cache/ and not at the filesystem root
• All existing cache tests continue to pass

Closes #669

[josephjclark]

Hi @lamine2000, thank you for raising this. I think there's just one necessary change but there are a few discussion points I'd like addressed.

I am not able to reproduce Mtuchi's original issue. It is quite old. Were you able to reproduce it with the CLI?

I appreciate the type fix on saveToCache. It is correct, but the PR comments are not. This untrue: This meant custom cache paths set by workspace projects were silently ignored when calling getCachePath from these functions. The option is carried through regardless of the typings.

Please check that AI generated PR descriptions are accurate! Misleading descriptions take more time to check

[josephjclark]

Not sure how often this problem occurs but I don't want to default to workflow here

A better solution would be

 const basePath = path.resolve(
    baseDir ?? process.cwd(),
    CACHE_DIR,
    workflowName,
  );

Where the undefined workflow name will simply be ignored by path.resolve (I think!)

[josephjclark]

Ah I see. Following your comment and checking the node docs, this doesn't actually work. A non string causes the throw. one comment coming in on the refactor

[josephjclark]

What is the different between getCachePath and getCacheRoot?

[josephjclark]

The old logic here, the reason we walk up through the folder structure, is to see if there's a parenting .gitignore file which we can edit. This would live at the root of the github repo, which we may or may not find.

This PR changes the approach to force a .gitignore file as a child of the cache folder.

I'm not against this. It might be a bit messy but I suppose it's not too big a deal. And I can see the original issue of writing a .gitignore file to root (although I can't reproduce this locally but that might be something in my local setup)

So Ok, let's do it!

[josephjclark]

Why is this test making such a distinction between workflow.js and expression.js? I don't think it materially affects the cache behaviour?

[josephjclark]

This test just ensures that the cache is written, not read. Which is fine - I'd just like the test name to be accurate.

[lamine2000]

Not with the CLI directly, no. But I traced both failure paths in the code and confirmed them.

For the `path.resolve` crash, this reproduces it:

```
node -e "const path = require('path'); path.resolve('/base', '.cli-cache', undefined)"

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined

```

`plan.workflow.name` is undefined when running a bare `.js` file, so that's exactly what was happening.

For the `.gitignore` at `/`, I confirmed it by reading the code. The old loop walked up the path one segment at a time looking for something ending in `.cli-cache`. If the cache path doesn't contain that string, it reaches root and writes there.

The two new tests cover both cases and fail against the original code, which felt like the most solid confirmation I could give.

On the description — you're right, I corrected it. The type fix doesn't affect runtime behaviour at all, the option was always passed through. I've updated the description to reflect that accurately.

And noted on AI-generated descriptions — I should have caught that before opening the PR. I'll make sure to verify every claim manually going forward.