v0.3.70 release - #327
Merged
Merged
Conversation
* feat: send X-Tower-Idempotency-Key on deploy Wire up the deploy command to send an idempotency key so consecutive deploys of unchanged source (e.g. to staging then production) collapse to a single AppVersion server-side instead of creating a new version each time. - Auto-populate the key from the git HEAD SHA when the working tree is clean; omit it on a dirty tree so provenance is never misrepresented. - Add --idempotency-key to override detection (useful for CI building outside a checkout) and --no-idempotency-key to opt out on a clean tree. - Print a hint when the server reuses an existing version so the user understands why no new version was created. - Apply the same git auto-detection to the MCP deploy tool. * test: add deploy idempotency key integration coverage BDD regression tests for the X-Tower-Idempotency-Key deploy behavior: explicit --idempotency-key, --no-idempotency-key opt-out, git auto-detect on a clean tree, header omission on a dirty tree, and the reuse hint on a repeat deploy with the same key. The mock API server now records the idempotency key seen on each deploy and reuses a stored (backdated) version when the same key recurs, exposing both via test-only inspection endpoints. * style: black-format deploy idempotency test files * chore: Cleanup to how the reuse hint is presented * chore: Don't really need to check output mode any longer * fix(test): match reuse-hint assertion to the new CLI wording Commit 560872b changed the deploy reuse hint from 'Reusing version ...' to 'No changes since commit ...' and updated the Rust unit test, but the behave step still asserted the old copy, failing the integration suite.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR replaces global CLI output state with ChangesCLI output and command wiring
Storage beta notices
Deploy idempotency
Binary build workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant DeployPipeline
participant API
CLI->>DeployPipeline: resolve idempotency key
DeployPipeline->>API: upload package with optional key
API-->>DeployPipeline: return new or cached version
DeployPipeline-->>CLI: display result or reuse hint
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/build-binaries.yml (1)
373-383: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a native container step instead of nested
docker-run-action.GitHub-hosted
ubuntu-24.04-armrunners already provide Docker; runningaddnab/docker-run-actiontoapk add rustand test the wheel duplicates functionality the runner already offers (flagged by zizmor assuperfluous-actions). Acontainer:step (or a plainrun:withdocker run) would be simpler and avoid an extra third-party action dependency. This mirrors the pre-existingmusllinuxjob's pattern, so it's a pre-existing style choice being replicated rather than a new regression.♻️ Illustrative alternative using a job-level container
musllinux-arm-test: needs: musllinux-arm runs-on: ubuntu-24.04-arm container: image: python:${{ matrix.python-version }}-alpine steps: - run: | apk add rust python -m venv .venv .venv/bin/pip3 install dist/*.whl --force-reinstall .venv/bin/${{ env.EXECUTABLE_NAME }} --help🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-binaries.yml around lines 373 - 383, Replace the addnab/docker-run-action step named “Test wheel” for the aarch64 musllinux target with a native container-based or plain docker run approach, matching the existing musllinux job pattern. Preserve the Alpine image, workspace access, Rust installation, virtualenv setup, wheel installation, and executable help check while removing the third-party action dependency.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tower-cmd/src/beta.rs`:
- Line 26: Update the user-facing STORAGE_BETA_MESSAGE constant to correct the
misspelled “featues” to “features,” preserving the rest of the beta notice
unchanged.
In `@crates/tower-cmd/src/deploy.rs`:
- Around line 253-258: Update the reuse hint message in the deployment flow to
avoid assuming sent_key is a git commit; use neutral wording that identifies it
as the key from the last deploy while preserving the key value and deployment
date.
In `@crates/tower-cmd/src/output.rs`:
- Around line 118-128: Update the non-JSON branch of muted so the dimmed message
passed to write includes a terminating newline, while preserving the existing
JSON behavior and styling.
---
Nitpick comments:
In @.github/workflows/build-binaries.yml:
- Around line 373-383: Replace the addnab/docker-run-action step named “Test
wheel” for the aarch64 musllinux target with a native container-based or plain
docker run approach, matching the existing musllinux job pattern. Preserve the
Alpine image, workspace access, Rust installation, virtualenv setup, wheel
installation, and executable help check while removing the third-party action
dependency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9fa4df15-12cb-45a3-ab2b-2826f04efd03
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.github/workflows/build-binaries.ymlcrates/config/Cargo.tomlcrates/config/src/lib.rscrates/config/src/session.rscrates/tower-cmd/src/beta.rscrates/tower-cmd/src/catalogs.rscrates/tower-cmd/src/deploy.rscrates/tower-cmd/src/lib.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/output.rscrates/tower-cmd/src/util/deploy.rscrates/tower-cmd/src/util/git.rscrates/tower-cmd/src/util/mod.rsplugin/skills/tower/SKILL.mdtests/integration/features/cli_deploy_idempotency.featuretests/integration/features/steps/cli_steps.pytests/mock-api-server/main.py
| } | ||
| } | ||
|
|
||
| pub(crate) const STORAGE_BETA_MESSAGE: &str = "Tower Storage is in beta. Core functionality is stable, but some featues and interfaces might change before general availability."; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the typo in the beta notice.
The user-facing message says “featues”; change it to “features” before release.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tower-cmd/src/beta.rs` at line 26, Update the user-facing
STORAGE_BETA_MESSAGE constant to correct the misspelled “featues” to “features,”
preserving the rest of the beta notice unchanged.
Comment on lines
+253
to
+258
| Some(format!( | ||
| "No changes since commit {} (deployed on {})", | ||
| sent_key, | ||
| util::dates::format(created_at), | ||
| )) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reuse hint wording assumes the key is a git commit.
"No changes since commit {sent_key} ..." is accurate when the key was auto-detected from git, but misleading when the user supplied an arbitrary value via --idempotency-key. Consider more neutral wording, e.g. "No changes since the last deploy with key {sent_key}", or branch the message based on whether the key came from git.
💬 Suggested tweak
Some(format!(
- "No changes since commit {} (deployed on {})",
+ "No changes since the last deploy with key `{}` (deployed on {})",
sent_key,
util::dates::format(created_at),
))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Some(format!( | |
| "No changes since commit {} (deployed on {})", | |
| sent_key, | |
| util::dates::format(created_at), | |
| )) | |
| } | |
| Some(format!( | |
| "No changes since the last deploy with key `{}` (deployed on {})", | |
| sent_key, | |
| util::dates::format(created_at), | |
| )) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tower-cmd/src/deploy.rs` around lines 253 - 258, Update the reuse hint
message in the deployment flow to avoid assuming sent_key is a git commit; use
neutral wording that identifies it as the key from the last deploy while
preserving the key value and deployment date.
* refactor(cli): replace the output globals with an explicit `Out` Where CLI output went was controlled by two mutable globals: an OUTPUT_MODE and a CURRENT_SENDER that the MCP server set and cleared around each tool call. Every output:: function read them to choose between stdout, JSON, and the MCP peer, so a command never said where its own output went. Pass the destination in as a value instead: commands take an Out and the writing functions become methods on it. The CLI builds one over stdout; an MCP tool builds one whose writer forwards each line to that call's channel. The sender and the notification flag live on the value, not in globals. * chore: rustfmt util/git.rs
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/tower-cmd/src/apps.rs (1)
407-412: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate error message across fatal/non-fatal branches.
out.error(&format!("Failed to stream run logs: {}", err))is emitted identically in both the fatal (return) and non-fatal (continue+backoff) branches. Hoisting it above theifavoids the duplication.♻️ Proposed refactor
Err(err) => { + out.error(&format!("Failed to stream run logs: {}", err)); if is_fatal_stream_error(&err) { - out.error(&format!("Failed to stream run logs: {}", err)); return; } - out.error(&format!("Failed to stream run logs: {}", err)); sleep(backoff).await; backoff = next_backoff(backoff); continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tower-cmd/src/apps.rs` around lines 407 - 412, In the error handling branch for the stream log operation, hoist the shared `out.error` call above the `is_fatal_stream_error(&err)` check so the message is emitted once. Preserve the fatal branch’s immediate return and the non-fatal branch’s existing continue/backoff behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tower-cmd/src/apps.rs`:
- Around line 120-148: Update do_logs so the non-follow describe_run_logs call
does not discard Err results; route failures through the module’s established
output error-handling path, while preserving the existing log-line iteration for
successful responses and the follow_logs branch.
In `@crates/tower-cmd/src/mcp.rs`:
- Around line 814-817: Update the deployment flow around deploy_from_dir and
do_deploy_package to preserve the returned deployment outcome instead of sending
output to Out::sink(). Propagate the reuse result through the MCP tool response
so reused deployments include the idempotency reuse hint rather than always
returning the generic success message.
In `@crates/tower-cmd/src/output.rs`:
- Around line 191-353: Update package_error, config_error, and
tower_error_and_die so --json requests emit exactly one structured error through
the Format/Out JSON path before exiting, rather than human-readable text or
multiple documents. Route usage errors occurring before Out initialization to
stderr, while preserving existing human-readable behavior for non-JSON output
and authentication-specific messaging.
---
Nitpick comments:
In `@crates/tower-cmd/src/apps.rs`:
- Around line 407-412: In the error handling branch for the stream log
operation, hoist the shared `out.error` call above the
`is_fatal_stream_error(&err)` check so the message is emitted once. Preserve the
fatal branch’s immediate return and the non-fatal branch’s existing
continue/backoff behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0ddf2a17-c402-4b7f-b64b-456bfc8d6f8c
📒 Files selected for processing (19)
crates/tower-cmd/src/apps.rscrates/tower-cmd/src/beta.rscrates/tower-cmd/src/catalogs.rscrates/tower-cmd/src/deploy.rscrates/tower-cmd/src/environments.rscrates/tower-cmd/src/lib.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/output.rscrates/tower-cmd/src/package.rscrates/tower-cmd/src/run.rscrates/tower-cmd/src/schedules.rscrates/tower-cmd/src/secrets.rscrates/tower-cmd/src/session.rscrates/tower-cmd/src/teams.rscrates/tower-cmd/src/util/apps.rscrates/tower-cmd/src/util/cmd.rscrates/tower-cmd/src/util/deploy.rscrates/tower-cmd/src/util/git.rscrates/tower-cmd/src/version.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/tower-cmd/src/util/deploy.rs
- crates/tower-cmd/src/beta.rs
- crates/tower-cmd/src/util/git.rs
- crates/tower-cmd/src/deploy.rs
Comment on lines
+120
to
148
| pub async fn do_logs(out: &output::Out, config: Config, cmd: &ArgMatches) { | ||
| let app_name_raw = cmd | ||
| .get_one::<String>("app_name") | ||
| .expect("app_name is required"); | ||
| let (name, seq) = if let Some((name, num_str)) = app_name_raw.split_once('#') { | ||
| let num = num_str | ||
| .parse::<i64>() | ||
| .unwrap_or_else(|_| output::die("Run number must be a number")); | ||
| .unwrap_or_else(|_| out.die("Run number must be a number")); | ||
| (name.to_string(), num) | ||
| } else { | ||
| let num = match cmd.get_one::<i64>("run_number").copied() { | ||
| Some(n) => n, | ||
| None => latest_run_number(&config, app_name_raw).await, | ||
| None => latest_run_number(out, &config, app_name_raw).await, | ||
| }; | ||
| (app_name_raw.clone(), num) | ||
| }; | ||
| let follow = cmd.get_one::<bool>("follow").copied().unwrap_or(false); | ||
|
|
||
| if follow { | ||
| follow_logs(config, name, seq).await; | ||
| follow_logs(out, config, name, seq).await; | ||
| return; | ||
| } | ||
|
|
||
| if let Ok(resp) = api::describe_run_logs(&config, &name, seq).await { | ||
| for line in resp.log_lines { | ||
| output::remote_log_event(&line); | ||
| out.remote_log_event(&line); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure and find relevant functions
ast-grep outline crates/tower-cmd/src/apps.rs --view expanded || true
echo '--- grep for key functions / patterns ---'
rg -n "do_logs|follow_logs|latest_run_number|tower_error_and_die|if let Ok\\(resp\\)" crates/tower-cmd/src/apps.rs
echo '--- relevant sections around do_logs ---'
sed -n '110,170p' crates/tower-cmd/src/apps.rs
echo '--- relevant sections around latest_run_number ---'
sed -n '250,340p' crates/tower-cmd/src/apps.rs
echo '--- relevant sections around follow_logs ---'
sed -n '340,430p' crates/tower-cmd/src/apps.rsRepository: tower/tower-cli
Length of output: 12864
Surface describe_run_logs errors in do_logs.
The non---follow branch drops Err from api::describe_run_logs, so a fetch failure exits cleanly with no output. Use the same error-handling path as the rest of this module.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tower-cmd/src/apps.rs` around lines 120 - 148, Update do_logs so the
non-follow describe_run_logs call does not discard Err results; route failures
through the module’s established output error-handling path, while preserving
the existing log-line iteration for successful responses and the follow_logs
branch.
Comment on lines
+814
to
+817
| // The tool builds its own result message, so the deploy's own progress is discarded. | ||
| let out = crate::output::Out::sink(); | ||
| match deploy::deploy_from_dir( | ||
| &out, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the deployment reuse result.
Line 815 discards deploy_from_dir output, including deploy::do_deploy_package’s idempotency reuse hint. MCP therefore always returns “Deploy completed successfully” even when the server reused an existing version. Capture or return the deployment outcome and include it in the tool result.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tower-cmd/src/mcp.rs` around lines 814 - 817, Update the deployment
flow around deploy_from_dir and do_deploy_package to preserve the returned
deployment outcome instead of sending output to Out::sink(). Propagate the reuse
result through the MCP tool response so reused deployments include the
idempotency reuse hint rather than always returning the generic success message.
Comment on lines
+191
to
353
| pub fn package_error(&self, err: tower_package::Error) { | ||
| let msg = match err { | ||
| tower_package::Error::NoManifest => "No manifest was found".to_string(), | ||
| tower_package::Error::InvalidManifest => { | ||
| "Invalid manifest was found or created".to_string() | ||
| } | ||
| tower_package::Error::InvalidPath => { | ||
| "There was a problem determining exactly where your Towerfile was stored on disk" | ||
| .to_string() | ||
| } | ||
| tower_package::Error::InvalidGlob { message } => { | ||
| format!("Invalid file glob pattern: {}", message) | ||
| } | ||
| tower_package::Error::InvalidTowerfile { message } => { | ||
| format!("Invalid Towerfile: {}", message) | ||
| } | ||
| tower_package::Error::MissingTowerfile => { | ||
| "No Towerfile was found in the target directory".to_string() | ||
| } | ||
| tower_package::Error::MissingRequiredAppField { field } => { | ||
| format!("Missing required app field `{}` in Towerfile", field) | ||
| } | ||
| tower_package::Error::Io { source } => format!("IO error: {}", source), | ||
| tower_package::Error::MissingScript { script } => { | ||
| format!("Script '{}' not found. Check that the 'script' field in your Towerfile points to a file that exists in your project.", script) | ||
| } | ||
| }; | ||
|
|
||
| pub fn title(text: &str) -> String { | ||
| text.bold().green().to_string() | ||
| } | ||
| let line = format!("{} {}\n", "Package error:".red(), msg); | ||
| self.write(&line); | ||
| } | ||
|
|
||
| pub fn placeholder(text: &str) -> String { | ||
| text.white().dimmed().italic().to_string() | ||
| } | ||
| pub fn config_error(&self, err: config::Error) { | ||
| let msg = match err { | ||
| config::Error::ConfigDirNotFound => "Config directory not found".to_string(), | ||
| config::Error::NoHomeDir => "No home directory found".to_string(), | ||
| config::Error::Io { ref source } => format!("IO error: {}", source), | ||
| config::Error::NoSession => "No session".to_string(), | ||
| config::Error::TeamNotFound { ref team_name } => { | ||
| format!("Team with name `{}` not found!", team_name) | ||
| } | ||
| config::Error::UnknownDescribeSessionValue { value: _ } => { | ||
| "An error occured while describing the session associated with the JWT you provided. Maybe your CLI is out of date?".to_string() | ||
| } | ||
| config::Error::DescribeSessionError { ref err } => { | ||
| format!("An error occured while describing the session associated with the JWT you provided: {}", err) | ||
| } | ||
| }; | ||
|
|
||
| pub fn paragraph(msg: &str) -> String { | ||
| msg.chars() | ||
| .collect::<Vec<char>>() | ||
| .chunks(78) | ||
| .map(|c| c.iter().collect::<String>()) | ||
| .map(|li| format!(" {}", li)) | ||
| .collect::<Vec<String>>() | ||
| .join("\n") | ||
| } | ||
| let line = format!("{} {}\n", "Config error:".red(), msg); | ||
| self.write(&line); | ||
| } | ||
|
|
||
| pub fn config_error(err: config::Error) { | ||
| let msg = match err { | ||
| config::Error::ConfigDirNotFound => "Config directory not found".to_string(), | ||
| config::Error::NoHomeDir => "No home directory found".to_string(), | ||
| config::Error::Io { ref source } => format!("IO error: {}", source), | ||
| config::Error::NoSession => "No session".to_string(), | ||
| config::Error::TeamNotFound { ref team_name } => { | ||
| format!("Team with name `{}` not found!", team_name) | ||
| } | ||
| config::Error::UnknownDescribeSessionValue { value: _ } => { | ||
| "An error occured while describing the session associated with the JWT you provided. Maybe your CLI is out of date?".to_string() | ||
| } | ||
| config::Error::DescribeSessionError { ref err } => { | ||
| format!("An error occured while describing the session associated with the JWT you provided: {}", err) | ||
| // Outputs both the model.detail and the model.errors fields in a human readable format. | ||
| fn output_full_error_details(&self, model: &ErrorModel) { | ||
| // Show the main detail message if available | ||
| if let Some(detail) = &model.detail { | ||
| self.write(&format!("\n{}\n", "Error details:".yellow())); | ||
| self.write(&format!("{}\n", detail.red())); | ||
| } | ||
|
|
||
| // Show any additional error details from the errors field | ||
| if let Some(errors) = &model.errors { | ||
| if !errors.is_empty() { | ||
| if model.detail.is_none() { | ||
| self.write(&format!("\n{}\n", "Error details:".yellow())); | ||
| } | ||
| for error in errors { | ||
| let msg = format!( | ||
| " • {}", | ||
| error.message.as_deref().unwrap_or("Unknown error") | ||
| ); | ||
| self.write(&format!("{}\n", msg.red())); | ||
| } | ||
| } | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| let line = format!("{} {}\n", "Config error:".red(), msg); | ||
| write(&line); | ||
| } | ||
| fn output_response_content_error<T>(&self, err: ResponseContent<T>) { | ||
| // Attempt to deserialize the error content into an ErrorModel. | ||
| let error_model = match serde_json::from_str::<ErrorModel>(&err.content) { | ||
| Ok(model) => { | ||
| debug!("Error model (status: {}): {:?}", err.status, model); | ||
| model | ||
| } | ||
| Err(e) => { | ||
| debug!("Failed to parse error content as JSON: {}", e); | ||
| debug!("Raw error content: {}", err.content); | ||
| // Show the raw error content if JSON parsing fails | ||
| self.write(&format!("\n{}\n", "API Error:".yellow())); | ||
| self.write(&format!("{}\n", err.content.red())); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| pub fn write(msg: &str) { | ||
| if get_output_mode().is_mcp() { | ||
| let clean_msg = msg.trim_end().to_string(); | ||
| send_to_current_sender(clean_msg); | ||
| } else { | ||
| write_to_stdout(msg); | ||
| match err.status { | ||
| StatusCode::CONFLICT => { | ||
| self.output_full_error_details(&error_model); | ||
| } | ||
| StatusCode::UNPROCESSABLE_ENTITY => { | ||
| self.output_full_error_details(&error_model); | ||
| } | ||
| StatusCode::INTERNAL_SERVER_ERROR => { | ||
| self.error( | ||
| "The Tower API encountered an internal error. Maybe try again later on.", | ||
| ); | ||
| } | ||
| StatusCode::NOT_FOUND => { | ||
| self.output_full_error_details(&error_model); | ||
| } | ||
| StatusCode::UNAUTHORIZED => { | ||
| self.error( | ||
| "You aren't authorized to do that! Are you logged in? Run `tower login` to login.", | ||
| ); | ||
| } | ||
| _ => { | ||
| if error_model.detail.is_none() && error_model.errors.is_none() { | ||
| self.error("The Tower API returned an error that the Tower CLI doesn't know what to do with! Maybe try again in a bit."); | ||
| } | ||
| self.output_full_error_details(&error_model); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub fn error(msg: &str) { | ||
| if get_output_mode().is_json() { | ||
| let response = serde_json::json!({ | ||
| "result": "error", | ||
| "message": msg | ||
| }); | ||
| json(&response); | ||
| } else { | ||
| let line = format!("{} {}\n", "Oh no!".red(), msg); | ||
| write(&line); | ||
| fn tower_error<T>(&self, err: ApiError<T>) { | ||
| match err { | ||
| ApiError::ResponseError(resp) => { | ||
| self.output_response_content_error(resp); | ||
| } | ||
| ApiError::Reqwest(e) => { | ||
| debug!("Reqwest error: {:?}", e); | ||
| self.error("The Tower CLI wasn't able to talk to the Tower API! Are you offline? Try again later."); | ||
| } | ||
| ApiError::Serde(e) => { | ||
| debug!("Serde error: {:?}", e); | ||
| self.error("The Tower API returned something that the Tower CLI didn't understand. Maybe you need to upgrade Tower CLI?"); | ||
| } | ||
| ApiError::Io(e) => { | ||
| debug!("Io error: {:?}", e); | ||
| self.error("An error happened while talking to the Tower API. You can try that again in a bit."); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub fn runtime_error(err: tower_runtime::errors::Error) { | ||
| let line = format!("{} {}\n", "Runtime Error:".red(), err.to_string()); | ||
| write(&line); | ||
| } | ||
| /// Handles Tower API errors with context-specific authentication messages. | ||
| /// If the error is a 401 Unauthorized, provides a helpful message mentioning | ||
| /// the operation that failed and suggests running 'tower login'. | ||
| /// Always exits the process with error code 1. | ||
| pub fn tower_error_and_die<T>(&self, err: ApiError<T>, operation: &str) -> ! { | ||
| // Check if this is an authentication error | ||
| if let ApiError::ResponseError(ref resp) = err { | ||
| if resp.status == StatusCode::UNAUTHORIZED { | ||
| self.die(&format!( | ||
| "{} because you are not logged into Tower. Please run 'tower login' first.", | ||
| operation | ||
| )); | ||
| } | ||
| } | ||
|
|
||
| // Outputs both the model.detail and the model.errors fields in a human readable format. | ||
| pub fn output_full_error_details(model: &ErrorModel) { | ||
| // Show the main detail message if available | ||
| if let Some(detail) = &model.detail { | ||
| write(&format!("\n{}\n", "Error details:".yellow())); | ||
| write(&format!("{}\n", detail.red())); | ||
| // Show the detailed error first | ||
| self.tower_error(err); | ||
| self.die(operation); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve a single parseable JSON error response.
These paths bypass Format, so API/package/config failures under --json emit human text or multiple documents. Render one structured error before exiting; send pre-Out usage errors to stderr.
Also applies to: 423-431, 603-611
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tower-cmd/src/output.rs` around lines 191 - 353, Update package_error,
config_error, and tower_error_and_die so --json requests emit exactly one
structured error through the Format/Out JSON path before exiting, rather than
human-readable text or multiple documents. Route usage errors occurring before
Out initialization to stderr, while preserving existing human-readable behavior
for non-JSON output and authentication-specific messaging.
* chore: Add support for querying Iceberg from the CLI * chore: Combine stages together in spinners, and require queries be read only * chore: Add explicit write mode for queries, so they can't be done accidentally * chore: Address some feedback from CodeRabbit * refactor: harden catalog SQL generation against injection Escaping now lives in SqlLiteral/SqlIdent Display wrappers instead of free functions each call site must remember, the table listing binds the catalog name as a real prepared-statement parameter, and attach_statements takes the vended credential Mode so call sites no longer pass an invertible read_only bool. * fix: convert nested DuckDB values to JSON and tilde-pin duckdb List/Array/Struct/Map/Union/Enum columns now render as proper JSON arrays and objects in --json output instead of Rust debug strings. The duckdb requirement uses a tilde constraint per upstream guidance, since the crate's second version component encodes the bundled DuckDB engine and a caret requirement would let cargo update swap engines silently.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tower-cmd/src/catalogs.rs`:
- Around line 372-376: Update redact_token to redact both the raw token and its
SQL-escaped form, where apostrophes are doubled as SqlLiteral does, while
preserving the empty-token behavior. Add a regression test with a token
containing an apostrophe and verify neither representation appears in the
resulting output.
- Around line 576-611: Update the Nanosecond handling in the timestamp and
Time64 conversion branches to preserve the nanosecond remainder instead of
reducing values directly to microseconds. Use chrono constructors that accept
seconds plus nanoseconds, including correct normalization for negative values
near the epoch. Apply the same fix to the additional conversion at the later
matching location, and add tests covering sub-microsecond and negative
nanosecond values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bad7fbff-54ad-4201-955a-4f19d266225f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.tomlcrates/tower-cmd/Cargo.tomlcrates/tower-cmd/src/catalogs.rscrates/tower-cmd/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/tower-cmd/src/lib.rs
Comment on lines
+372
to
+376
| fn redact_token(message: &str, token: &str) -> String { | ||
| if token.is_empty() { | ||
| return message.to_string(); | ||
| } | ||
| message.replace(token, "[REDACTED]") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact the SQL-escaped token form as well.
SqlLiteral changes ' to '', but redact_token only searches for the raw token. If DuckDB echoes setup SQL, secret'token appears as secret''token and can reach output.
Proposed fix
fn redact_token(message: &str, token: &str) -> String {
if token.is_empty() {
return message.to_string();
}
- message.replace(token, "[REDACTED]")
+ let escaped_token = token.replace('\'', "''");
+ message
+ .replace(&escaped_token, "[REDACTED]")
+ .replace(token, "[REDACTED]")
}Add a regression test using a token containing '.
Also applies to: 695-698, 1118-1130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tower-cmd/src/catalogs.rs` around lines 372 - 376, Update redact_token
to redact both the raw token and its SQL-escaped form, where apostrophes are
doubled as SqlLiteral does, while preserving the empty-token behavior. Add a
regression test with a token containing an apostrophe and verify neither
representation appears in the resulting output.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Line 7: Update the project version in pyproject.toml from the prerelease value
to the final 0.3.70 release, unless this PR is explicitly intended to publish a
release candidate; keep the package metadata consistent with the v0.3.70 release
objective.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fd19e6cb-dc0e-4ec2-b854-65d2f1c002f9
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
Cargo.tomlpyproject.toml
The bundled DuckDB build (libduckdb-sys) needs a musl C++ toolchain, which the stock Ubuntu runners don't provide. Run the musl dist jobs inside quay.io/pypa/musllinux_1_2 containers — the same images the maturin wheel builds already use — where gcc/g++ target musl natively.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
dist-workspace.toml (1)
52-52: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the musllinux images by digest.
These release-build containers currently resolve mutable tags, so identical source commits can use different toolchains over time. Pin both images to approved immutable
sha256digests and update them deliberately.Also applies to: 56-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dist-workspace.toml` at line 52, Update the release-build container entries in dist-workspace.toml, including both musllinux image definitions, to reference approved immutable sha256 digests instead of mutable image tags. Preserve the existing image repositories and host targets, and ensure both digests are deliberately selected and valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@dist-workspace.toml`:
- Line 52: Update the release-build container entries in dist-workspace.toml,
including both musllinux image definitions, to reference approved immutable
sha256 digests instead of mutable image tags. Preserve the existing image
repositories and host targets, and ensure both digests are deliberately selected
and valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a544735-489e-4a21-bf8e-72aa22f28d34
📒 Files selected for processing (1)
dist-workspace.toml
GitHub's JS actions (checkout, upload-artifact) only support Alpine containers on x64 runners. Switch the aarch64-unknown-linux-musl dist build to messense/rust-musl-cross:aarch64-musl, an Ubuntu-based arm64 image that carries the aarch64-linux-musl cross toolchain.
Recent npm versions refuse to publish a prerelease without an explicit --tag, which broke the wasm package publish for v0.3.70-rc.1. Tag prereleases as 'next' so they also never shadow 'latest'.
* chore: Fix the catalog show output and provide a full view * chore: Add `--full` to `catalogs show` so we can one-shot the whole environment
…tests (#331) * feat(catalogs): add DuckDB query sandbox primitives and adversarial tests Agent-issued catalog SQL is untrusted, so before an MCP query tool can run it we need the query path locked down and proof that the lockdown holds. This lands that foundation without wiring it to a caller yet. It adds a `sandbox` module with the gates a future `tower_catalogs_query` tool applies: reject write/DDL and multi-statement input (the multi-statement scanner skips separators inside strings and comments, which matters because duckdb-rs `prepare` executes every statement but the last as a side effect), and a session hardening set that disables local-filesystem access, blocks community extensions, and locks the configuration. The hardening disables only `LocalFileSystem`, so httpfs and the object-store reads an attached Iceberg catalog depends on keep working. `run_duckdb_query` gains an optional row cap that flags the result truncated, so a model cannot pull an unbounded table into memory or its context. The adversarial suite is the point: each test encodes an attack an agent query might attempt (data tampering, statement smuggling, host filesystem reads and writes, configuration escape, arbitrary extension loading, SSRF via table functions) and asserts the sandbox refuses it. A testcontainers MinIO test proves the same hardening does not break a real object-store Iceberg read while local access and config changes stay blocked, and self-skips when no Docker daemon is present so a plain `cargo test` still passes. * refactor(catalogs): extract Tower's DuckDB usage into a tower-duckdb crate and wire the sandbox in The sandbox primitives from the previous commit had no caller: the gates, the session hardening, and the row cap all lived in an inline `#[allow(dead_code)]` module in catalogs.rs, proven only by tests. This moves Tower's DuckDB usage into a dedicated `tower-duckdb` crate and makes `tower catalogs query` its first real consumer, so the hardening runs in production rather than sitting on a shelf. The crate owns the whole DuckDB surface: opening an in-memory Session, running trusted setup, locking the session down for untrusted SQL (Session::harden), and executing a query into JSON rows with an optional row cap. The static text gates (reject write/DDL, reject multi-statement) live in its `guard` module. The value conversion, the query execution, and the full adversarial + integration test suite move with it, so the security invariants are tested next to the code they protect. tower-cmd no longer depends on duckdb directly; it goes through the crate. `catalogs query` now splits by mode. Read mode is the default and the sandboxed path: the SQL is gated before it reaches DuckDB (a smuggled second statement would otherwise run as a side effect of prepare, and a write gets a clear message instead of a raw engine error), the session is hardened after attach, and rows are capped with a truncation notice. Write mode stays the trusted power-user opt-in and runs as before. Once this lands, #328 will layer the MCP `tower_catalogs_query` tool on top of the same crate. * fix(tower-duckdb): gate read-only queries with DuckDB's parser, not a keyword denylist The read-only gate checked only the first SQL keyword against a denylist. That is not a safe policy: a `--` comment ends at a carriage return as well as a newline in DuckDB, so a `-- x\rDROP …` payload looked empty to the scanner but parses as a DROP, and a statement that opens with an allowed keyword (a `WITH` CTE, for one) can still mutate. Addresses Konstantinos's review on #331. The gate now runs the SQL through DuckDB's own parser via `json_serialize_sql`, which parses without executing and serializes only SELECT statements, erroring on anything else. `classify_read_only` returns Allowed only for exactly one SELECT; everything else (writes, DDL, PRAGMA/SET, multi-statement, unparseable) is refused, fail-closed. This is an allowlist of what the executor will actually run, so the comment-terminator and CTE bypasses are caught, and the SQL is bound as a parameter rather than spliced into the parser query. The keyword denylist and the hand-rolled statement scanner are gone, along with their unit tests; new tests cover the single-SELECT allowlist, the `\r` smuggling case, the mutating-CTE case, and empty/multiple classification. * fix(tower-duckdb): bundle the json extension and skip iceberg test where it can't install The Windows test job failed because the read-only gate runs `json_serialize_sql`, which needs the `json` extension. It is not bundled, so DuckDB tried to auto-download and install it, and the install fails on the Windows runner ("Could not move file: Access is denied"). That is a real defect, not just a test problem: the gate would be broken for Windows users too. Enabling the duckdb `json` feature compiles the extension statically into the bundled build, so it is available with no autoload or network. Verified it works with `autoinstall_known_extensions` and `autoload_known_extensions` both off. The `iceberg_scan` regression test needs the `iceberg` extension, which has no such feature and must still be fetched at runtime. It now self-skips when that install cannot happen, the same way the object-store test skips without Docker, so the Windows job stops failing on an environment it can't satisfy while the test keeps running where it can. * test(tower-duckdb): co-locate read-only gate regression tests in guard.rs The gate's unit tests lived in lib.rs, away from the code they cover. This moves them next to `classify_read_only` in guard.rs and broadens them into a proper regression suite, so the parser-based read-only policy is pinned down where a reader of guard.rs will find it. The suite locks in the current DuckDB classification across the shapes that matter: the many forms of a single read (plain SELECT, CTEs, set operations, subqueries, VALUES/TABLE/FROM-first, DESCRIBE/SUMMARIZE/SHOW), that comments and whitespace and semicolons inside literals don't change the verdict, that writes and DDL and config/transaction/meta statements are rejected whatever the leading keyword, that a leading SELECT can't launder a trailing mutation (the `\r` comment-terminator and data-modifying-CTE bypasses), multi-statement and empty/comment-only classification, fail-closed rejection of unparseable input, and the shared-connection reuse path. It also documents one boundary explicitly: a SELECT that reads a local file or URL is Allowed by the gate because the gate classifies statement shape only; the session hardening is what refuses the read. The redundant gate tests are dropped from lib.rs; its adversarial suite keeps the shared `check` helper. * feat(tower-duckdb): close SELECT-shaped holes, expand the lockdown, and bound queries Parsing as a SELECT is a statement shape, not a read-only property, and the gate was treating the two as the same thing. Verified against the DuckDB we ship (v1.5.4): `SELECT nextval('s')` classifies as a plain SELECT and really does advance the sequence, and `query()`/`query_table()` classify as SELECTs while handing a string to the execution pipeline. Those are now refused by name from the parsed tree, walking `function_name` nodes rather than matching text, so comments and quoting cannot hide them. The session lockdown gains the settings it was missing: no implicit extension install or load, no unsigned extensions, secrets kept redacted, and optional memory and temp-size ceilings, with `lock_configuration` still last because it freezes every later SET. `Hardening` is now a struct so a caller that needs no object storage can set `deny_external_access` and close network egress. The default cannot: an attached Iceberg catalog is made of S3 reads, so egress stays open on the catalog path and the docs now say so plainly rather than implying the lockdown is total. A row cap turned out to bound almost nothing, since one row can carry a whole column (`string_agg`, `list`, `to_json`). Results are now bounded by rows, total bytes, and wall-clock time together, via `Limits`; the byte ceiling is what actually holds, and the timeout exists because DuckDB has no statement timeout of its own, so it is enforced by interrupting the connection. `QueryResult.truncated` carries which ceiling was hit so the CLI can say which one. The docs now state that none of this is the security boundary: the read-only credential and the READ_ONLY attach are, and everything here is defence in depth in front of them. Tests pin each judgement against the shipped DuckDB, including canaries that fail loudly on upgrade if data-modifying CTEs start parsing as SELECTs or `prepare` stops executing leading statements. * feat(catalogs): add --max-rows to override the query result ceiling Read queries are bounded by default so a runaway result cannot flood a terminal or a model's context, but there was no way to ask for more. That makes the bound a wall rather than a default: an agent (or a person) with a legitimate need for a large extract had no option short of --write, which vends read-write credentials for what is still only a read. --max-rows sets the row ceiling and lifts the size ceiling with it, because a caller who asks for a million rows should not then be cut short by a byte budget they never set. --max-rows 0 removes the ceilings entirely. The help text says plainly that a large result can exhaust memory, since that is the trade being made. The read-only gate and the session hardening are untouched: this widens how much data a read may return, not what a query is allowed to do. The decision is factored into `query_limits` so it is unit-tested directly rather than only through argument parsing. * fix(tower-duckdb): make the function gate fail-closed and the ceilings real Review found three ways the sandbox did less than it claimed, and two ways the tests hid it. The function denylist was not fail-closed. `json_execute_serialized_sql` runs whatever SQL it is handed and ships with the very `json` extension this crate enables to build the gate, so the gate supplied its own bypass: the outer statement is an ordinary SELECT and the effectful call hides in a string. `enable_logging`, `checkpoint`, `setseed` and friends were allowed for the same reason, that naming dangerous functions one at a time can never be complete. The gate now allowlists the table-function position, which is where the danger lives, so an unknown function is refused rather than waved through. Scalars are too numerous to allowlist, so it asks the engine instead: DuckDB's `has_side_effects` flags `nextval`, `setseed` and the rest, and keeps up with new versions on its own. It is NULL for every table function, which is why those are allowlisted rather than queried. An explicit list covers what neither reaches: the dynamic-SQL executors and the effectful table functions. This tightens the read path, since `read_csv`/`read_parquet` and the rest of that family are no longer permitted; a catalog query reads base tables, which are not functions. The byte ceiling measured a row after admitting it, so `SELECT repeat('x', 5e7)` returned fifty megabytes and labelled it truncated. It now measures first and withholds the row, which means a single over-budget row yields an empty truncated result. That is blunt but honest, and it is what the ceiling was for. Because that still only bounds what the caller is handed, the read path now hardens with `Hardening::agent()`, which sets an engine memory limit for what a query spends before the first row exists. The SSRF test passed for a reason production does not share: its helper never loaded `httpfs`, so there was no HTTP filesystem to block. It now loads the same extensions `attach_statements` does, and counts connections to a listener it owns rather than trusting an error string, with a control run so it cannot pass vacuously. Note the hardening blocks these today only because disabling LocalFileSystem breaks path resolution first; egress is still not closed by design, and that gap is documented rather than papered over. Also: the MinIO image is pinned to a release tag instead of `latest` and only skips when Docker is genuinely unreachable, so a registry problem fails loudly instead of silently disabling a security test; the iceberg skip is narrowed the same way; and the workspace `rust-version` is corrected to 1.88, which the toolchain and the dependency tree have required for some time. * fix(tower-duckdb): close the replacement-scan SSRF path CI found on Linux CI caught what my local testing could not. The SSRF I reported as non-reproducible does reproduce on Linux: three hardened queries reached the probe listener there. The block I measured on macOS was incidental, disabling LocalFileSystem breaks HTTP path resolution before the HTTP filesystem is consulted, and Linux does not do that. Relying on it was the mistake; the reviewer was right that the hardening is not an egress control. Two holes are closed at the gate, which is platform-independent. `SELECT * FROM 'http://…'` is a replacement scan: it parses as an ordinary base table with the URL as its name, so the table-function allowlist never saw it. Base-table references that name a file or a URL are now refused outright, with their own verdict so the error can say what is actually wrong. `iceberg_scan` is out of the allowlist. It takes a location, so allowing it left a way to reach internal services even with the rest of the family refused. Every remaining entry is a pure generator that accepts no path and no URL, which is the property to preserve: adding anything there that takes a location reopens this. The test now mirrors production by running the gate first and executing only what it allows, because asserting that hardening alone blocks the network was asserting something this crate's own documentation says is untrue. A companion test records that hardening does not close egress, so the next person does not rediscover it in CI. Separately, the MinIO test failed on Windows because MinIO publishes Linux images only, and my previous change had turned that into a hard failure. It now skips when no Linux container runtime is available, decided from the platform rather than inferred from a pull error, and still fails loudly for anything else.
* chore: Fix the catalog show output and provide a full view * chore: Add `--full` to `catalogs show` so we can one-shot the whole environment * chore: Add `catalog facts` to tower-cli * refactor(catalogs): rename catalog `facts` command to `knowledge` `facts` collides with fact tables in dimensional modeling — confusing right where this lives, on a data catalog. Rename the CLI surface and tower-cmd internals to `knowledge`. The generated tower-api client still calls them `facts` since the backend is unchanged.
* Add query support to local MCP server Adds a tower_catalogs_query MCP tool that runs one read-only SQL statement against a Tower-managed storage catalog, and extends tower_catalogs_show to list the catalog's namespaces and tables. Agent queries go through the tower-duckdb guard (single read-only statement, denied functions and file/URL table references rejected), run hardened (Hardening::agent()), and are capped by Limits::agent() (1000 rows / 1 MiB / 60s). Rows come back as positional arrays so duplicate column names survive. * test(catalogs): gated MCP catalog e2e scenarios Behave scenarios for the MCP catalog tools: show lists tables, a read-only query succeeds, writes and multi-statement SQL are rejected. Skipped unless TOWER_TEST_CATALOG (and a real TOWER_URL) is set, since they exercise the real attach -> Iceberg -> query path. * style: apply black formatting to mcp_steps.py
Co-authored-by: bradhe <310958+bradhe@users.noreply.github.com>
konstantinoscs
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
--idempotency-keyand--no-idempotency-key, with auto-use of the current git commit SHA when the tree is clean.tower deploycommand reference with the new idempotency options.