Skip to content

Put terminate behind a lambda - #17

Merged
ethanholz merged 2 commits into
omsf-eco-infra:mainfrom
dwhswenson:cli-lambda-terminate
Jul 16, 2026
Merged

Put terminate behind a lambda#17
ethanholz merged 2 commits into
omsf-eco-infra:mainfrom
dwhswenson:cli-lambda-terminate

Conversation

@dwhswenson

Copy link
Copy Markdown
Member

This adds the terminate command to the CLI lambda.

Assisted-by: Codex.app:GPT-5.4

This adds the `terminate` command to the CLI lambda.

Assisted-by: Codex.app:GPT-5.4
@codecov

codecov Bot commented May 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.79%. Comparing base (2ff1ed6) to head (a2f7590).

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #17   +/-   ##
=======================================
  Coverage   99.79%   99.79%           
=======================================
  Files          18       19    +1     
  Lines        3372     3386   +14     
=======================================
+ Hits         3365     3379   +14     
  Misses          7        7           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dwhswenson
dwhswenson marked this pull request as ready for review July 16, 2026 18:47
@dwhswenson

Copy link
Copy Markdown
Member Author

Tested in a sandbox deployment -- looks like it works!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the terminate CLI command to the Lambda-backed CLI path by introducing a shared terminate command module, wiring it into the Click CLI and the CLI Lambda router, and updating tests/infrastructure to support the new action.

Changes:

  • Add terminate command implementation shared between local CLI and CLI Lambda (src/devbox/commands/terminate.py) and register it in the Lambda dispatch table.
  • Update Click terminate entrypoint to invoke the remote command helper and adjust CLI tests accordingly.
  • Extend IAM policy statements for the Lambda to include TerminateInstances, and add/adjust docs and dependencies.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/test_cli.py Updates CLI tests to patch the new run_terminate_command path and updated help/argument naming.
tests/commands/test_terminate.py Adds unit tests for terminate command helpers and Lambda-side handler behavior.
src/devbox/commands/terminate.py New shared terminate command module (client payload, remote invocation, Lambda handler).
src/devbox/cli.py Routes devbox terminate through run_terminate_command with normalized --param-prefix.
src/devbox/cli_protocol.py Adds CliAction.TERMINATE to the shared action enum.
src/devbox/cli_lambda/app.py Registers terminate handler in the Lambda action dispatch table.
pyproject.toml Adds a pinned charset-normalizer runtime dependency.
PLAN-cli-lambda.md Updates Phase 2 terminate plan/contract text and validation log notes.
modules/cli-lambda/main.tf Adds a dedicated IAM statement for terminate action permissions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_cli.py
Comment on lines 1051 to 1053
def test_consistent_error_exit_codes(
self, mock_console_class, mock_manager_class, mock_run_status_command, command, args
self, mock_console_class, mock_run_terminate_command, mock_run_status_command, command, args
):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leaving this as-is for future extensibility (although I might refactor these tests at some point anyway)

Comment thread pyproject.toml Outdated
Comment thread pyproject.toml Outdated
]
dependencies = [
"boto3>=1.26.0",
"charset-normalizer==3.4.0", # pin needed to fix issue with request

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, I couldn't figure out why requests had this problem. IIRC, it looked like it had something to do with someone not wanting to use chardet, but for now "because of requests" is the best I can do.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@dwhswenson
dwhswenson requested a review from ethanholz July 16, 2026 19:13

@ethanholz ethanholz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to merge, but is the plan to move all functionality of the CLI to a lambda?

"""Enumeration of supported Lambda-backed CLI actions."""

STATUS = "status"
TERMINATE = "terminate"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I correct in understanding that the CLI operation for creating EC2 instances are not yet behind a lambda?

@dwhswenson

Copy link
Copy Markdown
Member Author

Yes; step by step moving things to lambda so that the CLI user only needs permissions to get the lambda URL from an SSM parameter and to invoke the lambda.

Moving one CLI subcommand per PR to keep the changesets simpler.

@ethanholz

Copy link
Copy Markdown
Contributor

Sounds good, just wanted to make sure that was the case and I wasn't missing something! I am going to merge this one!

@ethanholz
ethanholz merged commit 4ae5ffe into omsf-eco-infra:main Jul 16, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants