Repository navigation
feat: add useLocalCli input to run the project-local CLI - #11
AntoineMontane wants to merge 1 commit into
Conversation
Projects that already depend on cdktn-cli can set `useLocalCli: true` to run that CLI from node_modules via `npx --no cdktn` instead of fetching a version with npx. The CLI version then follows the project's lockfile, in sync with local runs and dependency updates, and no extra package is downloaded per run. `--no` makes npx fail rather than silently download a CLI when it isn't installed. No version input is required in this mode; if one is set, it is ignored with a warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
e3b265f to
5d54b90
Compare
so0k
left a comment
There was a problem hiding this comment.
Thanks for this, running the project's own CLI is a good fit for lockfile and Renovate workflows. #12 merged first and touched the same buildNpxArgs and test file, so I rebased this branch onto main for you (5d54b90), keeping both features. With useLocalCli, an apply now runs npx --no cdktn apply <stack> --auto-approve --outputs-file <file>. dist/ was rebuilt after npm ci, and npm run all passes (41 tests). Please pull before pushing again (git fetch && git reset --hard origin/feat/use-local-cli).
A few things need changing before we merge; details are in the inline comments:
- Legacy
cdktf-cliprojects can't useuseLocalCli.npx --no cdktnlooks for acdktncommand, butcdktf-cli@0.21.0installs only acdktfcommand and thecdktnlibrary package installs none. Such projects fail withnpm error could not determine executable to run. - The warnings are confusing in local mode. The ignored input isn't named, and the cdktf deprecation and "using cdktnVersion" warnings still fire for inputs local mode ignores.
- The docs overstate "project-local". If no local CLI is installed, npx falls back to a globally installed one, because
--noonly blocks downloads. Also, a--yesincustomNpxArgsre-enables downloads. - Tests. We reviewed them against our test-audit guidelines (
.claude/skills/test-audit/SKILL.md):- Keep "runs the project-local cdktn without fetching it".
- Fold the other two into it as a table-driven local-mode test (one mostly duplicates an existing test, and the other locks in point 1).
- Add the cases that aren't covered:
getInputs()acceptinguseLocalCliwith no version input, the new warnings, and local mode passing--outputs-fileon apply.
| args.push(...splitArgs(inputs.customNpxArgs)) | ||
| args.push(`${cli.pkg}@${cli.version}`) | ||
| if (inputs.useLocalCli) { | ||
| args.push('cdktn') |
There was a problem hiding this comment.
This always runs the cdktn command, which only cdktn-cli installs. cdktf-cli@0.21.0 installs only cdktf, and the cdktn library package installs none. So a project still on the legacy CLI (useLocalCli: true with cdktfVersion) fails with npm error could not determine executable to run.
Passing the package name instead works: npx --no <pkg> runs that package's command from the local node_modules, including a hoisted root node_modules when run from a subdirectory. I checked this with npx 11.16 for both cdktn-cli and cdktf-cli. You can't reuse resolveCli(inputs).pkg as is, though: with no version input it returns cdktf-cli. Something like this:
// The deprecated cdktfVersion still selects the legacy package; otherwise the project's cdktn-cli runs.
const pkg = inputs.cdktfVersion && !inputs.cdktnVersion ? 'cdktf-cli' : 'cdktn-cli'
args.push(pkg)The version inputs would then choose which local package runs rather than being ignored, so the docs and warning need to match.
If you'd rather guarantee the CLI really is project-local, an alternative is to skip npx and resolve the package yourself: createRequire(join(workingDirectory, 'package.json')).resolve(`${pkg}/package.json`), then run its bin entry with node. That rules out the global fallback and lets the action fail early with a clear message naming the missing package. It's more code than npx --no <pkg>, so either is fine with us.
| @@ -55,8 +55,16 @@ export function getInputs(): ActionInputs { | |||
|
|
|||
| const cdktnVersion = core.getInput('cdktnVersion') | |||
| const cdktfVersion = core.getInput('cdktfVersion') | |||
There was a problem hiding this comment.
None of the getInputs() changes here are tested yet: accepting useLocalCli with no version input, and the new warning. The run() tests from #12 mock core.getInput and core.getBooleanInput, so a case there can assert both that run() doesn't throw without a version and what core.warning was called with.
| throw new Error( | ||
| 'Provide a "cdktnVersion" (or the deprecated "cdktfVersion") input, or set "useLocalCli".', | ||
| ) | ||
| } |
There was a problem hiding this comment.
Two things in local mode:
- This warning doesn't say which input is ignored. Name it, e.g.
"cdktfVersion" is ignored because "useLocalCli" is set., or change the wording to match whatever selection rule you adopt inbuildNpxArgs. - The checks below still run, so a local-mode user also gets "cdktfVersion is deprecated" or "using cdktnVersion" for inputs the action doesn't use. Skip them when
useLocalCliis set.
| required: false | ||
| useLocalCli: | ||
| description: >- | ||
| Run the CLI installed in the project's node_modules (`npx --no cdktn`) |
There was a problem hiding this comment.
Two small accuracy points for this description and the README row:
npx --noonly blocks downloads. If no local CLI is installed, npx falls back to a globally installed one rather than failing, so "comes from the project's lockfile" isn't guaranteed. One sentence noting this is enough, unless you resolve the package explicitly as suggested onsrc/action.ts.- A
--yesincustomNpxArgscomes after--noand re-enables downloads. A short note is enough.
The npx --no cdktn text will also need updating if the command changes for cdktf projects.
| expect(args).toEqual(['--no', 'cdktn', 'plan', 'dev']) | ||
| }) | ||
|
|
||
| it('ignores version inputs when running the local CLI', () => { |
There was a problem hiding this comment.
The name promises "ignored, with a warning", but the warning lives in getInputs() and isn't asserted here. The test also makes cdktfVersion + local mode → cdktn the intended behaviour, which is the cdktf gap on src/action.ts. I'd fold it into a table-driven local-mode test, with a row for the legacy package once the selection rule is settled.
| expect(args).toEqual(['--no', 'cdktn', 'synth']) | ||
| }) | ||
|
|
||
| it('threads custom npx args before the local CLI', () => { |
There was a problem hiding this comment.
This mostly repeats "threads custom npx and cdktn args": splitArgs(customNpxArgs) runs before the local/remote branch, so this only catches a refactor that moves the push into one branch. It's cheaper as a row in the same table-driven local-mode test, and that table should also have a row for local mode + apply passing --outputs-file, which nothing covers yet.
Adds a
useLocalCliinput. When it'strue, the action runs thecdktnCLI the project already has innode_modules(npx --no cdktn …) instead of fetchingcdktn-cli@<cdktnVersion>withnpx --yes.Why: projects that already depend on
cdktn-cliusually want CI to run the same version as local runs and their lockfile, kept up to date by Dependabot or Renovate, with no extra package download per run.hashicorp/terraform-cdk-actionhad no way to do that. Some forks added a similar flag, for exampleuseLocalCdktf.Behaviour:
npx --nomakes the step fail ifcdktnisn't installed, rather than silently downloading some other version. The dependency install has to run before this step.cdktnVersionorcdktfVersionis also set, it's ignored, with a warning.customNpxArgsandcdktnArgsare still passed as before.false, so existing workflows behave the same.Changes:
action.yml,models.tsandinputs.ts;buildNpxArgs;dist/.npm run allpasses: check, typecheck, 35 tests and the bundle.🤖 Generated with Claude Code