Repository navigation
fix(cli): release terraform state lock when interrupting diff/deploy - #284
Conversation
|
Thanks for taking this on. I ran the PR head through our local termless Exact setup/checks I ran: cd /data/repos/cdktn-e2e
# build/publish/install the cdk-terrain PR head into a local sandbox CLI
CDKTN_MONOREPO=/data/repos/hermes-pr-reviewer/worktrees/cdk-terrain/open-constructs__cdk-terrain/pr-284 pnpm provision cdktn-prhead
# static check for the e2e repo
pnpm typecheck
# run the relevant termless interrupt/lock tests against the provisioned PR-head CLI
CLI_ID=cdktn-prhead pnpm exec vitest run tests/diff-lock-release.test.ts tests/ctrl-c-teardown.test.tsObserved results:
The important failure is the unmerged PR #3 e2e test: That test uses a deterministic in-process Terraform HTTP backend, so it directly records the backend I also saw this existing interrupt e2e fail against the same provisioned PR-head CLI: That one hung after Ctrl-C at the Approve/Dismiss/Stop prompt ( Could you share how you validated the fix locally, and whether you tried the |
|
I have tested this locally. This is not interrupting Terraform at the approve prompt, but during the plan or apply process. |
|
@X-Guardian I think this looks good, but curious what the behavior of a 2nd Ctrl-C press is. My expectation is that it would abort gracefully shutdown. |
|
I tested this and both Ctrl-C's get passed through to Terraform which then responds with: |
🤖 Release PR — merge to cut a new release. Kept open and rebased as commits land on `main`. --- <details><summary>0.23.4</summary> ## [0.23.4](v0.23.3...v0.23.4) (2026-07-02) ### Features * **cli:** validate Terraform CLI feature versions ([#237](#237)) ([32d434c](32d434c)) * declared Terraform/OpenTofu target versions (validation foundation) ([#269](#269)) ([0aa2343](0aa2343)) * **lib:** S3-backend - Add `useLockfile` field and update `dynamodbTable` deprecated JSDoc ([#234](#234)) ([32a408f](32a408f)) * **lib:** validate Fn usage against Terraform/OpenTofu versions ([#268](#268)) ([cdf8115](cdf8115)) ### Bug Fixes * **cli:** don't mistake terraform error output for a missing-variable prompt ([#267](#267)) ([0ea946f](0ea946f)) * **cli:** release terraform state lock when interrupting diff/deploy ([#284](#284)) ([de617db](de617db)) * **cli:** resolve prebuilt Java provider versions via repo1.maven.org ([#285](#285)) ([d5af07d](d5af07d)) * **gha:** Adding CHANGELOG.md to prettier ignore list ([#258](#258)) ([5160469](5160469)) * **gha:** pull jsii-terraform CI image from GHCR instead of Docker Hub ([#261](#261)) ([f19d34b](f19d34b)) * **provider-generator:** copy only publishable files into the jsii compile bundle ([#292](#292)) ([562d4c7](562d4c7)) * **provider-generator:** match shorthand provider sources against OpenTofu schema keys ([#293](#293)) ([fda0103](fda0103)) * **release:** add Changelog title for correct release-please insertion ([#255](#255)) ([004818a](004818a)) * **release:** drop devEngines.packageManager that broke npm publish ([ef508a6](ef508a6)) * **tools:** repair generate-function-bindings tool ([#273](#273)) ([821e1cf](821e1cf)) ### Miscellaneous Chores * Add Gradle to Mise config ([#271](#271)) ([1578270](1578270)) * add pnpm to mise ([#266](#266)) ([48ab91f](48ab91f)) * add uv to mise ([#260](#260)) ([163cf3d](163cf3d)) * **deps:** bump nrwl/nx-set-shas from 38457b511e60ee4e1da09255b658e480748a1af4 to afb73a62d26e41464e9254689e1fd6122ee683c1 ([#236](#236)) ([b5ac018](b5ac018)) * **deps:** bump the github-actions-backward-compatible group across 1 directory with 7 updates ([#235](#235)) ([de67d9f](de67d9f)) * **deps:** replace uuid dependency with node:crypto.randomUUID ([#291](#291)) ([6d3657a](6d3657a)) * ESLint ignore test edge-provider-bindings ([#270](#270)) ([ba407bb](ba407bb)) * Fix Verdaccio logging config ([#272](#272)) ([aded64b](aded64b)) * **gha:** parallelize provider binding generation for documentation examples ([#244](#244)) ([0a9c955](0a9c955)) * Migrate package manager from yarn to pnpm ([#170](#170)) ([d23da4e](d23da4e)) * migrate planning artifacts to cdktn-planning [skip ci] ([968c358](968c358)) * **release:** add release-please automation for versioning ([#243](#243)) ([c759eec](c759eec)) * Remove dependency-pinner tool ([#250](#250)) ([42c27d5](42c27d5)) * Remove unused dependencies ([#281](#281)) ([c7de65e](c7de65e)) * **tests:** run verdaccio in-process ([#248](#248)) ([c17677d](c17677d)) * Update JSII version constraint ([#245](#245)) ([d89558f](d89558f)) * Update scheduled workflows to only run on open-constructs/cdk-terrain ([#256](#256)) ([ba408cf](ba408cf)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: open-constructs-cdktn[bot] <291052431+open-constructs-cdktn[bot]@users.noreply.github.com>
|
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
Related issue
Fixes #283
Description
Interrupting
cdktn difforcdktn deploy(Ctrl-C) left terraform's state lock held, so the next run failed withError acquiring the state lock.The cause was that the CLI's abort handler tore terraform down instead of letting it exit on its own. Terraform shares the CLI's process group, so Ctrl-C already reaches it directly and starts its graceful shutdown (which releases the lock) — but the CLI killed it before that shutdown could finish. The fix is to stop tearing terraform down on abort and instead wait for it to exit:
diff/plan (
@cdktn/commons/src/util.ts):exec()no longer forwards the abort tochild_process.spawn'ssignaloption, which killed the child with SIGTERM and rejected the promise immediately (syntheticAbortError) before terraform had exited — so terraform never ran its graceful shutdown and the lock leaked. It now settles on the child's ownclose, so the run resolves only once terraform has exited and released the lock.deploy/destroy (
@cdktn/cli-core/src/lib/models/deploy-machine.ts): the xstate machine previously went straight to the finalstoppedstate onSTOP, killing the pty and resolving the run before terraform had finished shutting down. It now passes through an intermediatestoppingstate that marks the run cancelled and waits for terraform's naturalEXITEDbefore reachingstopped. The pty stays alive throughout, so terraform exits on its own and the run only resolves afterwards.Why no extra signal is needed: every abort trigger in the codebase (
runCdktfProject,watch) originates fromprocess.on(SIGINT/SIGTERM/SIGQUIT), so terraform — sharing the process group — always receives the signal directly. There is no abort path that requires the CLI to deliver the signal itself.Verification: confirmed after the fix that a single Ctrl-C produces only
Interrupt received... Gracefully shutting down...and terraform exits cleanly on its own. The full lock-release path (interrupt a locking operation, then re-run) still needs a final check against a backend that locks state.Checklist