Conversation
…y found' (#232) Previously, all std::io::Error instances were silently converted to ConfigDirNotFound, which displayed as 'No home directory found'. This masked real errors like read-only filesystems, permission denied, disk full, etc. Now IO errors are preserved and surfaced with their actual message, making failures much easier to diagnose. Co-authored-by: Brad Heller <brad@tower.dev> Co-authored-by: Vim <121349594+sammuti@users.noreply.github.com>
|
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:
📝 WalkthroughWalkthroughAdd explicit IO error variant to config errors; support API-key auth in config; change Towerfile parameter API to upsert/remove; extend MCP with edit/remove flows and shared mutate helper; introduce retry/failure run models and status variants; update logging target logic; add guest/usage API endpoints and clients; bump versions and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant MCP_Server
participant TowerfileStore
participant Filesystem
Client->>MCP_Server: POST tower_file_add/edit/remove_parameter
note right of MCP_Server: validate request (hidden vs default), prepare mutation
MCP_Server->>TowerfileStore: modify_towerfile(mutation_closure)
TowerfileStore->>Filesystem: read Towerfile file
Filesystem-->>TowerfileStore: return Towerfile content
TowerfileStore->>TowerfileStore: perform set/remove/upsert mutation
TowerfileStore->>Filesystem: write updated Towerfile
Filesystem-->>TowerfileStore: write acknowledgement
TowerfileStore-->>MCP_Server: return success or error
MCP_Server-->>Client: "Towerfile updated" / error text
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
coderabbitai
Bot
requested review from
codingcyclist,
giray123,
jo-sm and
sammuti
![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)
crates/tower-telemetry/src/logging.rs (1)
138-138: Prefer explicit level matching over enum-order comparisons.The comparison
*level < LogLevel::Warnat line 184 works correctly today (Debug and Info are declared before Warn), but relying on enum variant order creates a hidden coupling. Reordering variants or adding new ones could silently change behavior.Replace the
PartialOrdderivation with explicit matching via a helper method:♻️ Proposed refactor
-#[derive(PartialEq, PartialOrd)] +#[derive(Clone, Copy, PartialEq, Eq)] pub enum LogLevel { Debug, Info, Warn, Error, } + +impl LogLevel { + fn should_show_target(self) -> bool { + matches!(self, LogLevel::Debug | LogLevel::Info) + } +} @@ - let with_target = *level < LogLevel::Warn; + let with_target = level.should_show_target();This makes the intent explicit and protects against future variant changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tower-telemetry/src/logging.rs` at line 138, Remove the PartialOrd derive from the LogLevel enum and replace the enum-order comparison with an explicit helper on LogLevel; add an impl LogLevel { fn is_below_warn(&self) -> bool { matches!(self, LogLevel::Debug | LogLevel::Info) } } (or similarly named predicate) and update all call sites that currently use comparisons like `*level < LogLevel::Warn` to use `level.is_below_warn()` (or the chosen predicate) instead so behavior is explicit and resistant to reordering or adding variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tower-cmd/src/mcp.rs`:
- Around line 831-843: When adding a parameter in the add-parameter path, do not
default missing description/default to empty strings for visible parameters;
instead enforce that when request.hidden is false both request.description and
request.default are Some and non-empty and return Self::text_error(...) if not.
Update the code around modify_towerfile so that before calling tf.set_parameter
you validate request.hidden, request.description, and request.default (check
.is_some() and that .as_ref().unwrap().is_empty() is false), and then pass
description: request.description.clone().unwrap() and default:
request.default.clone().unwrap() into Parameter (remove unwrap_or_default
usage). Reference: request.hidden, request.description, request.default,
modify_towerfile, tf.set_parameter, and Parameter.
- Around line 855-867: When renaming a parameter in the update path, prevent
collisions by checking whether request.new_name (when Some and different from
the original name) already exists in the target collection before calling
tf.set_parameter; if a different parameter with that name exists, return an
error (e.g., "parameter '{new_name}' already exists"). Locate the block around
the existing lookup and Parameter construction (references:
tf.parameters.iter(), existing, request.new_name, Parameter, tf.set_parameter)
and add an early check that searches tf.parameters for request.new_name and
returns an Err when a collision is found.
In `@crates/tower-runtime/src/lib.rs`:
- Line 59: The PR marks Status::Failed as terminal via is_terminal(), but the
monitor loop and post-monitor result matcher in crates/tower-cmd/src/run.rs
don't handle it explicitly, causing infinite polling and improper error
handling; update the match in the monitor loop (the polling/monitoring match
that currently falls through with `_ => continue`) to include a case for
Status::Failed that breaks/returns to stop monitoring, and update the
post-monitor result match (the one that currently falls through to the generic
error arm) to add an explicit Status::Failed branch that logs/returns the
appropriate failure error; reference Status::Failed, is_terminal(), the monitor
loop match and the post-monitor result match when making the changes.
---
Nitpick comments:
In `@crates/tower-telemetry/src/logging.rs`:
- Line 138: Remove the PartialOrd derive from the LogLevel enum and replace the
enum-order comparison with an explicit helper on LogLevel; add an impl LogLevel
{ fn is_below_warn(&self) -> bool { matches!(self, LogLevel::Debug |
LogLevel::Info) } } (or similarly named predicate) and update all call sites
that currently use comparisons like `*level < LogLevel::Warn` to use
`level.is_below_warn()` (or the chosen predicate) instead so behavior is
explicit and resistant to reordering or adding variants.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 904dcf42-72f0-42c6-a780-bf6a870d97b5
📒 Files selected for processing (10)
crates/config/src/error.rscrates/config/src/lib.rscrates/config/src/towerfile.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/output.rscrates/tower-runtime/src/auto_cleanup.rscrates/tower-runtime/src/lib.rscrates/tower-runtime/src/local.rscrates/tower-runtime/tests/local_test.rscrates/tower-telemetry/src/logging.rs
Comment on lines
+831
to
+843
| if request.hidden && request.default.is_some() { | ||
| return Self::text_error("hidden and default are mutually exclusive".into()); | ||
| } | ||
| let name = request.name.clone(); | ||
| Self::modify_towerfile(&request.common, |tf| { | ||
| tf.set_parameter(&name, Parameter { | ||
| name: name.clone(), | ||
| description: request.description.unwrap_or_default(), | ||
| default: request.default.unwrap_or_default(), | ||
| hidden: request.hidden, | ||
| }); | ||
| Ok(format!("Added parameter '{name}'")) | ||
| }) |
There was a problem hiding this comment.
Enforce required fields for visible parameters.
Line 838 and Line 839 default missing values to empty strings, so hidden = false can still create effectively blank parameters even though the tool contract says description/default are required for regular parameters.
Proposed fix
if request.hidden && request.default.is_some() {
return Self::text_error("hidden and default are mutually exclusive".into());
}
+ if !request.hidden {
+ if request
+ .description
+ .as_deref()
+ .map(str::trim)
+ .unwrap_or("")
+ .is_empty()
+ {
+ return Self::text_error(
+ "description is required when hidden=false".into(),
+ );
+ }
+ if request.default.is_none() {
+ return Self::text_error("default is required when hidden=false".into());
+ }
+ }
let name = request.name.clone();📝 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
| if request.hidden && request.default.is_some() { | |
| return Self::text_error("hidden and default are mutually exclusive".into()); | |
| } | |
| let name = request.name.clone(); | |
| Self::modify_towerfile(&request.common, |tf| { | |
| tf.set_parameter(&name, Parameter { | |
| name: name.clone(), | |
| description: request.description.unwrap_or_default(), | |
| default: request.default.unwrap_or_default(), | |
| hidden: request.hidden, | |
| }); | |
| Ok(format!("Added parameter '{name}'")) | |
| }) | |
| if request.hidden && request.default.is_some() { | |
| return Self::text_error("hidden and default are mutually exclusive".into()); | |
| } | |
| if !request.hidden { | |
| if request | |
| .description | |
| .as_deref() | |
| .map(str::trim) | |
| .unwrap_or("") | |
| .is_empty() | |
| { | |
| return Self::text_error( | |
| "description is required when hidden=false".into(), | |
| ); | |
| } | |
| if request.default.is_none() { | |
| return Self::text_error("default is required when hidden=false".into()); | |
| } | |
| } | |
| let name = request.name.clone(); | |
| Self::modify_towerfile(&request.common, |tf| { | |
| tf.set_parameter(&name, Parameter { | |
| name: name.clone(), | |
| description: request.description.unwrap_or_default(), | |
| default: request.default.unwrap_or_default(), | |
| hidden: request.hidden, | |
| }); | |
| Ok(format!("Added parameter '{name}'")) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tower-cmd/src/mcp.rs` around lines 831 - 843, When adding a parameter
in the add-parameter path, do not default missing description/default to empty
strings for visible parameters; instead enforce that when request.hidden is
false both request.description and request.default are Some and non-empty and
return Self::text_error(...) if not. Update the code around modify_towerfile so
that before calling tf.set_parameter you validate request.hidden,
request.description, and request.default (check .is_some() and that
.as_ref().unwrap().is_empty() is false), and then pass description:
request.description.clone().unwrap() and default:
request.default.clone().unwrap() into Parameter (remove unwrap_or_default
usage). Reference: request.hidden, request.description, request.default,
modify_towerfile, tf.set_parameter, and Parameter.
giray123
approved these changes
Mar 31, 2026
* chore: regen clients * fix incompatibilities with the new API
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/tower-cmd/src/api.rs (1)
841-850: Extract duplicatedRunParametermapping into a helper.
The two blocks are identical; centralizing this avoids future divergence between create/update schedule paths.♻️ Suggested refactor
+fn map_run_parameters( + parameters: Option<HashMap<String, String>>, +) -> Option<Vec<RunParameter>> { + parameters.map(|params| { + params + .into_iter() + .map(|(key, value)| RunParameter { + name: key, + value, + hidden: None, + }) + .collect() + }) +} + pub async fn create_schedule( @@ - let run_parameters = parameters.map(|params| { - params - .into_iter() - .map(|(key, value)| RunParameter { - name: key, - value, - hidden: None, - }) - .collect() - }); + let run_parameters = map_run_parameters(parameters); @@ pub async fn update_schedule( @@ - let run_parameters = parameters.map(|params| { - params - .into_iter() - .map(|(key, value)| RunParameter { - name: key, - value, - hidden: None, - }) - .collect() - }); + let run_parameters = map_run_parameters(parameters);Also applies to: 879-888
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tower-cmd/src/api.rs` around lines 841 - 850, The mapping from parameters -> Vec<RunParameter> is duplicated; extract it into a helper function (e.g., fn map_run_parameters(parameters: Option<ParametersType>) -> Option<Vec<RunParameter>>) and replace both inline blocks that build run_parameters with calls to that helper. Locate the duplicated mapping around the run_parameters variable and the equivalent block at lines handling create/update schedule paths and use the new map_run_parameters helper to centralize constructing RunParameter { name, value, hidden: None }.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/tower-cmd/src/api.rs`:
- Around line 841-850: The mapping from parameters -> Vec<RunParameter> is
duplicated; extract it into a helper function (e.g., fn
map_run_parameters(parameters: Option<ParametersType>) ->
Option<Vec<RunParameter>>) and replace both inline blocks that build
run_parameters with calls to that helper. Locate the duplicated mapping around
the run_parameters variable and the equivalent block at lines handling
create/update schedule paths and use the new map_run_parameters helper to
centralize constructing RunParameter { name, value, hidden: None }.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48854873-3436-4d4f-81d6-a2ba51659c05
📒 Files selected for processing (271)
crates/tower-api/README.mdcrates/tower-api/src/apis/configuration.rscrates/tower-api/src/apis/default_api.rscrates/tower-api/src/apis/feature_flags_api.rscrates/tower-api/src/models/account.rscrates/tower-api/src/models/acknowledge_alert_response.rscrates/tower-api/src/models/acknowledge_all_alerts_response.rscrates/tower-api/src/models/alert.rscrates/tower-api/src/models/api_key.rscrates/tower-api/src/models/app.rscrates/tower-api/src/models/app_statistics.rscrates/tower-api/src/models/app_summary.rscrates/tower-api/src/models/app_version.rscrates/tower-api/src/models/authentication_context.rscrates/tower-api/src/models/batch_schedule_params.rscrates/tower-api/src/models/batch_schedule_response.rscrates/tower-api/src/models/cancel_run_response.rscrates/tower-api/src/models/catalog.rscrates/tower-api/src/models/catalog_property.rscrates/tower-api/src/models/claim_device_login_ticket_params.rscrates/tower-api/src/models/claim_device_login_ticket_response.rscrates/tower-api/src/models/create_account_params.rscrates/tower-api/src/models/create_account_params_flags_struct.rscrates/tower-api/src/models/create_account_response.rscrates/tower-api/src/models/create_api_key_params.rscrates/tower-api/src/models/create_api_key_response.rscrates/tower-api/src/models/create_app_params.rscrates/tower-api/src/models/create_app_response.rscrates/tower-api/src/models/create_authenticator_params.rscrates/tower-api/src/models/create_authenticator_response.rscrates/tower-api/src/models/create_catalog_params.rscrates/tower-api/src/models/create_catalog_response.rscrates/tower-api/src/models/create_device_login_ticket_response.rscrates/tower-api/src/models/create_environment_params.rscrates/tower-api/src/models/create_environment_response.rscrates/tower-api/src/models/create_guest_params.rscrates/tower-api/src/models/create_guest_response.rscrates/tower-api/src/models/create_password_reset_params.rscrates/tower-api/src/models/create_password_reset_response.rscrates/tower-api/src/models/create_sandbox_secrets_params.rscrates/tower-api/src/models/create_sandbox_secrets_response.rscrates/tower-api/src/models/create_schedule_params.rscrates/tower-api/src/models/create_schedule_response.rscrates/tower-api/src/models/create_secret_params.rscrates/tower-api/src/models/create_secret_response.rscrates/tower-api/src/models/create_session_params.rscrates/tower-api/src/models/create_session_response.rscrates/tower-api/src/models/create_team_params.rscrates/tower-api/src/models/create_team_response.rscrates/tower-api/src/models/create_webhook_params.rscrates/tower-api/src/models/create_webhook_response.rscrates/tower-api/src/models/delete_api_key_params.rscrates/tower-api/src/models/delete_api_key_response.rscrates/tower-api/src/models/delete_app_response.rscrates/tower-api/src/models/delete_authenticator_params.rscrates/tower-api/src/models/delete_authenticator_response.rscrates/tower-api/src/models/delete_catalog_response.rscrates/tower-api/src/models/delete_guest_output_body.rscrates/tower-api/src/models/delete_schedule_params.rscrates/tower-api/src/models/delete_schedule_response.rscrates/tower-api/src/models/delete_secret_response.rscrates/tower-api/src/models/delete_session_params.rscrates/tower-api/src/models/delete_session_response.rscrates/tower-api/src/models/delete_team_invitation_params.rscrates/tower-api/src/models/delete_team_invitation_response.rscrates/tower-api/src/models/delete_team_params.rscrates/tower-api/src/models/delete_team_response.rscrates/tower-api/src/models/delete_webhook_response.rscrates/tower-api/src/models/deploy_app_request.rscrates/tower-api/src/models/deploy_app_response.rscrates/tower-api/src/models/describe_account_body.rscrates/tower-api/src/models/describe_app_response.rscrates/tower-api/src/models/describe_app_version_response.rscrates/tower-api/src/models/describe_authentication_context_body.rscrates/tower-api/src/models/describe_device_login_session_response.rscrates/tower-api/src/models/describe_email_preferences_body.rscrates/tower-api/src/models/describe_plan_response.rscrates/tower-api/src/models/describe_run_graph_response.rscrates/tower-api/src/models/describe_run_links.rscrates/tower-api/src/models/describe_run_logs_response.rscrates/tower-api/src/models/describe_run_response.rscrates/tower-api/src/models/describe_secrets_key_response.rscrates/tower-api/src/models/describe_session_response.rscrates/tower-api/src/models/describe_team_response.rscrates/tower-api/src/models/describe_webhook_response.rscrates/tower-api/src/models/email_subscriptions.rscrates/tower-api/src/models/encrypted_catalog_property.rscrates/tower-api/src/models/environment.rscrates/tower-api/src/models/error_detail.rscrates/tower-api/src/models/error_model.rscrates/tower-api/src/models/event_alert.rscrates/tower-api/src/models/event_error.rscrates/tower-api/src/models/event_log.rscrates/tower-api/src/models/event_shouldertap.rscrates/tower-api/src/models/event_warning.rscrates/tower-api/src/models/export_catalogs_params.rscrates/tower-api/src/models/export_catalogs_response.rscrates/tower-api/src/models/export_secrets_params.rscrates/tower-api/src/models/export_secrets_response.rscrates/tower-api/src/models/exported_catalog.rscrates/tower-api/src/models/exported_catalog_property.rscrates/tower-api/src/models/exported_secret.rscrates/tower-api/src/models/feature.rscrates/tower-api/src/models/featurebase_identity.rscrates/tower-api/src/models/generate_app_statistics_response.rscrates/tower-api/src/models/generate_authenticator_response.rscrates/tower-api/src/models/generate_run_statistics_response.rscrates/tower-api/src/models/generate_runner_credentials_response.rscrates/tower-api/src/models/get_feature_flag_response_body.rscrates/tower-api/src/models/guest.rscrates/tower-api/src/models/invite_team_member_params.rscrates/tower-api/src/models/invite_team_member_response.rscrates/tower-api/src/models/leave_team_response.rscrates/tower-api/src/models/list_alerts_response.rscrates/tower-api/src/models/list_api_keys_response.rscrates/tower-api/src/models/list_app_environments_response.rscrates/tower-api/src/models/list_app_versions_response.rscrates/tower-api/src/models/list_apps_response.rscrates/tower-api/src/models/list_authenticators_response.rscrates/tower-api/src/models/list_catalogs_response.rscrates/tower-api/src/models/list_environments_response.rscrates/tower-api/src/models/list_guests_response.rscrates/tower-api/src/models/list_my_team_invitations_response.rscrates/tower-api/src/models/list_runners_response.rscrates/tower-api/src/models/list_runs_response.rscrates/tower-api/src/models/list_schedules_response.rscrates/tower-api/src/models/list_secret_environments_response.rscrates/tower-api/src/models/list_secrets_response.rscrates/tower-api/src/models/list_team_invitations_response.rscrates/tower-api/src/models/list_team_members_response.rscrates/tower-api/src/models/list_teams_response.rscrates/tower-api/src/models/list_webhooks_response.rscrates/tower-api/src/models/mod.rscrates/tower-api/src/models/organization.rscrates/tower-api/src/models/organization_usage.rscrates/tower-api/src/models/pagination.rscrates/tower-api/src/models/parameter.rscrates/tower-api/src/models/plan.rscrates/tower-api/src/models/refresh_session_params.rscrates/tower-api/src/models/refresh_session_response.rscrates/tower-api/src/models/regenerate_guest_login_url_params.rscrates/tower-api/src/models/regenerate_guest_login_url_response.rscrates/tower-api/src/models/remove_team_member_params.rscrates/tower-api/src/models/remove_team_member_response.rscrates/tower-api/src/models/resend_team_invitation_params.rscrates/tower-api/src/models/resend_team_invitation_response.rscrates/tower-api/src/models/run.rscrates/tower-api/src/models/run_app_initiator_data.rscrates/tower-api/src/models/run_app_params.rscrates/tower-api/src/models/run_app_response.rscrates/tower-api/src/models/run_attempt.rscrates/tower-api/src/models/run_failure_alert.rscrates/tower-api/src/models/run_graph_node.rscrates/tower-api/src/models/run_graph_run_id.rscrates/tower-api/src/models/run_initiator.rscrates/tower-api/src/models/run_initiator_details.rscrates/tower-api/src/models/run_log_line.rscrates/tower-api/src/models/run_parameter.rscrates/tower-api/src/models/run_results.rscrates/tower-api/src/models/run_retry_policy.rscrates/tower-api/src/models/run_run_initiator_details.rscrates/tower-api/src/models/run_statistics.rscrates/tower-api/src/models/run_timeseries_point.rscrates/tower-api/src/models/runner.rscrates/tower-api/src/models/runner_credentials.rscrates/tower-api/src/models/schedule.rscrates/tower-api/src/models/schedule_run_initiator_details.rscrates/tower-api/src/models/search_runs_response.rscrates/tower-api/src/models/secret.rscrates/tower-api/src/models/server_sent_events_inner.rscrates/tower-api/src/models/server_sent_events_inner_1.rscrates/tower-api/src/models/server_sent_events_inner_2.rscrates/tower-api/src/models/session.rscrates/tower-api/src/models/shoulder_tap.rscrates/tower-api/src/models/sse_warning.rscrates/tower-api/src/models/statistics_settings.rscrates/tower-api/src/models/team.rscrates/tower-api/src/models/team_invitation.rscrates/tower-api/src/models/team_membership.rscrates/tower-api/src/models/test_webhook_response.rscrates/tower-api/src/models/token.rscrates/tower-api/src/models/unverified_authenticator.rscrates/tower-api/src/models/update_account_params.rscrates/tower-api/src/models/update_account_response.rscrates/tower-api/src/models/update_app_params.rscrates/tower-api/src/models/update_app_response.rscrates/tower-api/src/models/update_catalog_params.rscrates/tower-api/src/models/update_catalog_response.rscrates/tower-api/src/models/update_email_preferences_body.rscrates/tower-api/src/models/update_environment_params.rscrates/tower-api/src/models/update_environment_response.rscrates/tower-api/src/models/update_my_team_invitation_params.rscrates/tower-api/src/models/update_my_team_invitation_response.rscrates/tower-api/src/models/update_organization_params.rscrates/tower-api/src/models/update_organization_response.rscrates/tower-api/src/models/update_password_reset_params.rscrates/tower-api/src/models/update_password_reset_response.rscrates/tower-api/src/models/update_plan_params.rscrates/tower-api/src/models/update_plan_response.rscrates/tower-api/src/models/update_schedule_params.rscrates/tower-api/src/models/update_schedule_response.rscrates/tower-api/src/models/update_secret_params.rscrates/tower-api/src/models/update_secret_response.rscrates/tower-api/src/models/update_team_member_params.rscrates/tower-api/src/models/update_team_member_response.rscrates/tower-api/src/models/update_team_params.rscrates/tower-api/src/models/update_team_response.rscrates/tower-api/src/models/update_user_params.rscrates/tower-api/src/models/update_user_response.rscrates/tower-api/src/models/update_webhook_params.rscrates/tower-api/src/models/update_webhook_response.rscrates/tower-api/src/models/usage_limit.rscrates/tower-api/src/models/user.rscrates/tower-api/src/models/verified_authenticator.rscrates/tower-api/src/models/verify_email_params.rscrates/tower-api/src/models/verify_email_response.rscrates/tower-api/src/models/webhook.rscrates/tower-cmd/src/api.rscrates/tower-cmd/src/apps.rscrates/tower-cmd/src/mcp.rssrc/tower/tower_api_client/api/default/create_guest.pysrc/tower/tower_api_client/api/default/delete_guest.pysrc/tower/tower_api_client/api/default/describe_organization_usage.pysrc/tower/tower_api_client/api/default/leave_team.pysrc/tower/tower_api_client/api/default/list_guests.pysrc/tower/tower_api_client/api/default/list_teams.pysrc/tower/tower_api_client/api/default/regenerate_guest_login_url.pysrc/tower/tower_api_client/api/default/update_organization.pysrc/tower/tower_api_client/models/__init__.pysrc/tower/tower_api_client/models/account.pysrc/tower/tower_api_client/models/api_key.pysrc/tower/tower_api_client/models/app.pysrc/tower/tower_api_client/models/create_api_key_params.pysrc/tower/tower_api_client/models/create_guest_params.pysrc/tower/tower_api_client/models/create_guest_response.pysrc/tower/tower_api_client/models/create_schedule_params.pysrc/tower/tower_api_client/models/delete_guest_output_body.pysrc/tower/tower_api_client/models/describe_run_links.pysrc/tower/tower_api_client/models/generate_run_statistics_status_item.pysrc/tower/tower_api_client/models/guest.pysrc/tower/tower_api_client/models/invite_team_member_params.pysrc/tower/tower_api_client/models/invite_team_member_params_role.pysrc/tower/tower_api_client/models/list_guests_response.pysrc/tower/tower_api_client/models/list_runs_status_item.pysrc/tower/tower_api_client/models/organization_usage.pysrc/tower/tower_api_client/models/parameter.pysrc/tower/tower_api_client/models/regenerate_guest_login_url_params.pysrc/tower/tower_api_client/models/regenerate_guest_login_url_response.pysrc/tower/tower_api_client/models/run.pysrc/tower/tower_api_client/models/run_app_params.pysrc/tower/tower_api_client/models/run_attempt.pysrc/tower/tower_api_client/models/run_log_line.pysrc/tower/tower_api_client/models/run_parameter.pysrc/tower/tower_api_client/models/run_results.pysrc/tower/tower_api_client/models/run_retry_policy.pysrc/tower/tower_api_client/models/run_status.pysrc/tower/tower_api_client/models/run_timeseries_point.pysrc/tower/tower_api_client/models/runner.pysrc/tower/tower_api_client/models/schedule.pysrc/tower/tower_api_client/models/search_runs_status_item.pysrc/tower/tower_api_client/models/team.pysrc/tower/tower_api_client/models/team_invitation.pysrc/tower/tower_api_client/models/update_account_params.pysrc/tower/tower_api_client/models/update_account_params_execution_region.pysrc/tower/tower_api_client/models/update_app_params.pysrc/tower/tower_api_client/models/update_schedule_params.pysrc/tower/tower_api_client/models/update_team_params.pysrc/tower/tower_api_client/models/update_team_params_execution_region.pysrc/tower/tower_api_client/models/usage_limit.pytests/mock-api-server/main.pytests/tower/test_client.py
✅ Files skipped from review due to trivial changes (190)
- crates/tower-api/src/models/delete_team_invitation_params.rs
- crates/tower-api/src/models/create_secret_response.rs
- crates/tower-api/src/models/describe_app_version_response.rs
- crates/tower-api/src/models/create_session_params.rs
- crates/tower-api/src/models/create_catalog_params.rs
- crates/tower-api/src/models/create_account_response.rs
- crates/tower-api/src/models/authentication_context.rs
- crates/tower-api/src/models/catalog.rs
- crates/tower-api/src/models/create_environment_response.rs
- crates/tower-api/src/models/delete_catalog_response.rs
- crates/tower-api/src/models/delete_api_key_response.rs
- crates/tower-api/src/models/create_webhook_response.rs
- crates/tower-api/src/models/create_api_key_response.rs
- crates/tower-api/src/models/create_secret_params.rs
- crates/tower-api/src/models/create_device_login_ticket_response.rs
- crates/tower-api/src/models/describe_plan_response.rs
- crates/tower-api/src/models/delete_authenticator_params.rs
- crates/tower-api/src/models/delete_team_response.rs
- crates/tower-api/src/models/delete_schedule_response.rs
- crates/tower-api/src/models/encrypted_catalog_property.rs
- crates/tower-api/src/models/create_session_response.rs
- crates/tower-api/src/models/email_subscriptions.rs
- crates/tower-api/src/models/delete_team_params.rs
- crates/tower-api/src/models/describe_session_response.rs
- crates/tower-api/src/models/describe_email_preferences_body.rs
- crates/tower-api/src/models/event_alert.rs
- crates/tower-api/src/models/describe_team_response.rs
- crates/tower-api/src/models/delete_authenticator_response.rs
- crates/tower-api/src/models/event_warning.rs
- crates/tower-api/src/models/describe_account_body.rs
- crates/tower-api/src/models/deploy_app_request.rs
- crates/tower-api/src/models/error_model.rs
- crates/tower-api/src/models/describe_run_graph_response.rs
- crates/tower-api/src/models/error_detail.rs
- crates/tower-api/src/models/get_feature_flag_response_body.rs
- crates/tower-api/src/models/feature.rs
- crates/tower-api/src/models/describe_run_response.rs
- crates/tower-api/src/models/exported_catalog.rs
- crates/tower-api/src/models/environment.rs
- crates/tower-api/src/models/invite_team_member_response.rs
- crates/tower-api/src/models/generate_app_statistics_response.rs
- crates/tower-api/src/models/event_error.rs
- crates/tower-api/src/models/event_log.rs
- crates/tower-api/src/models/list_environments_response.rs
- crates/tower-api/src/models/export_catalogs_params.rs
- crates/tower-api/src/models/featurebase_identity.rs
- crates/tower-api/src/models/list_team_invitations_response.rs
- crates/tower-api/src/models/list_teams_response.rs
- crates/tower-api/src/models/run_app_initiator_data.rs
- crates/tower-api/src/models/list_apps_response.rs
- crates/tower-api/src/models/list_app_versions_response.rs
- crates/tower-api/src/models/plan.rs
- crates/tower-api/src/models/list_team_members_response.rs
- crates/tower-api/src/models/run_failure_alert.rs
- crates/tower-api/src/models/secret.rs
- crates/tower-api/src/models/list_alerts_response.rs
- crates/tower-api/src/models/generate_run_statistics_response.rs
- crates/tower-api/src/models/list_runs_response.rs
- crates/tower-api/src/models/list_secret_environments_response.rs
- crates/tower-api/src/models/event_shouldertap.rs
- crates/tower-api/src/models/list_api_keys_response.rs
- crates/tower-api/src/models/session.rs
- crates/tower-api/src/models/server_sent_events_inner_2.rs
- crates/tower-api/src/models/run_graph_node.rs
- crates/tower-api/src/models/run_statistics.rs
- crates/tower-api/src/models/search_runs_response.rs
- crates/tower-api/src/models/shoulder_tap.rs
- crates/tower-api/src/models/refresh_session_params.rs
- crates/tower-api/src/models/list_app_environments_response.rs
- crates/tower-api/src/models/update_my_team_invitation_response.rs
- crates/tower-api/src/models/resend_team_invitation_response.rs
- crates/tower-api/src/models/runner_credentials.rs
- crates/tower-api/src/models/update_account_response.rs
- crates/tower-api/src/models/generate_runner_credentials_response.rs
- crates/tower-api/src/models/update_environment_response.rs
- crates/tower-api/src/models/run_initiator.rs
- crates/tower-api/src/models/unverified_authenticator.rs
- crates/tower-api/src/models/list_runners_response.rs
- crates/tower-api/src/models/resend_team_invitation_params.rs
- crates/tower-api/src/models/update_environment_params.rs
- crates/tower-api/src/models/server_sent_events_inner.rs
- crates/tower-api/src/models/token.rs
- crates/tower-api/src/models/statistics_settings.rs
- crates/tower-api/src/models/schedule_run_initiator_details.rs
- crates/tower-api/src/models/update_email_preferences_body.rs
- crates/tower-api/src/models/update_secret_params.rs
- crates/tower-api/src/models/update_secret_response.rs
- crates/tower-api/src/models/user.rs
- crates/tower-api/src/models/update_plan_response.rs
- crates/tower-api/src/models/update_password_reset_response.rs
- crates/tower-api/src/models/update_webhook_params.rs
- crates/tower-api/src/models/update_schedule_response.rs
- crates/tower-api/src/models/update_app_response.rs
- crates/tower-api/src/models/verified_authenticator.rs
- crates/tower-api/src/models/test_webhook_response.rs
- crates/tower-api/src/models/update_catalog_params.rs
- crates/tower-api/src/models/update_my_team_invitation_params.rs
- crates/tower-api/src/models/server_sent_events_inner_1.rs
- crates/tower-api/src/models/update_plan_params.rs
- crates/tower-api/src/models/update_team_response.rs
- crates/tower-api/src/models/update_catalog_response.rs
- crates/tower-api/src/models/webhook.rs
- crates/tower-api/src/models/update_team_member_response.rs
- crates/tower-api/src/models/verify_email_params.rs
- crates/tower-api/src/models/alert.rs
- crates/tower-api/src/models/describe_authentication_context_body.rs
- crates/tower-api/src/apis/configuration.rs
- crates/tower-api/src/models/app_version.rs
- crates/tower-api/src/models/create_account_params.rs
- crates/tower-api/src/models/batch_schedule_response.rs
- crates/tower-api/src/models/acknowledge_all_alerts_response.rs
- crates/tower-api/src/models/create_catalog_response.rs
- crates/tower-api/src/models/acknowledge_alert_response.rs
- crates/tower-api/src/models/create_app_params.rs
- crates/tower-api/src/models/claim_device_login_ticket_response.rs
- crates/tower-api/src/models/create_app_response.rs
- crates/tower-api/src/models/create_schedule_response.rs
- crates/tower-api/src/models/create_account_params_flags_struct.rs
- crates/tower-api/src/models/create_sandbox_secrets_response.rs
- crates/tower-api/src/models/create_authenticator_params.rs
- crates/tower-api/src/models/create_password_reset_params.rs
- crates/tower-api/src/models/create_environment_params.rs
- crates/tower-api/src/models/create_webhook_params.rs
- crates/tower-api/src/models/create_sandbox_secrets_params.rs
- crates/tower-api/src/models/create_team_params.rs
- crates/tower-api/src/models/app_summary.rs
- crates/tower-api/src/models/create_authenticator_response.rs
- crates/tower-api/src/models/delete_schedule_params.rs
- crates/tower-api/src/models/delete_secret_response.rs
- crates/tower-api/src/models/create_team_response.rs
- crates/tower-api/src/models/delete_session_params.rs
- crates/tower-api/src/models/delete_app_response.rs
- crates/tower-api/src/models/export_catalogs_response.rs
- crates/tower-api/src/models/describe_app_response.rs
- crates/tower-api/src/models/deploy_app_response.rs
- crates/tower-api/src/models/describe_device_login_session_response.rs
- crates/tower-api/src/models/delete_session_response.rs
- crates/tower-api/src/models/export_secrets_response.rs
- crates/tower-api/src/models/describe_secrets_key_response.rs
- crates/tower-api/src/models/leave_team_response.rs
- crates/tower-api/src/models/generate_authenticator_response.rs
- crates/tower-api/src/models/exported_catalog_property.rs
- crates/tower-api/src/models/remove_team_member_params.rs
- crates/tower-api/src/models/list_my_team_invitations_response.rs
- crates/tower-api/src/models/list_authenticators_response.rs
- crates/tower-api/src/models/refresh_session_response.rs
- crates/tower-api/src/models/export_secrets_params.rs
- crates/tower-api/src/models/list_webhooks_response.rs
- crates/tower-api/src/models/list_secrets_response.rs
- crates/tower-api/src/models/run_initiator_details.rs
- crates/tower-api/src/models/run_graph_run_id.rs
- crates/tower-api/src/models/run_run_initiator_details.rs
- crates/tower-api/src/models/list_catalogs_response.rs
- crates/tower-api/src/models/update_organization_response.rs
- crates/tower-api/src/models/sse_warning.rs
- crates/tower-api/src/models/team_membership.rs
- crates/tower-api/src/models/update_user_response.rs
- crates/tower-api/src/models/update_organization_params.rs
- crates/tower-api/src/models/update_password_reset_params.rs
- crates/tower-api/src/models/app_statistics.rs
- crates/tower-api/src/models/batch_schedule_params.rs
- crates/tower-api/src/models/cancel_run_response.rs
- crates/tower-api/src/models/update_team_member_params.rs
- crates/tower-api/src/models/remove_team_member_response.rs
- src/tower/tower_api_client/api/default/leave_team.py
- crates/tower-api/src/models/verify_email_response.rs
- src/tower/tower_api_client/models/generate_run_statistics_status_item.py
- src/tower/tower_api_client/models/search_runs_status_item.py
- src/tower/tower_api_client/models/run_status.py
- crates/tower-api/src/models/organization.rs
- crates/tower-api/src/models/create_guest_params.rs
- crates/tower-api/src/models/delete_webhook_response.rs
- crates/tower-api/src/models/regenerate_guest_login_url_params.rs
- crates/tower-api/src/models/create_password_reset_response.rs
- crates/tower-api/src/models/claim_device_login_ticket_params.rs
- crates/tower-api/src/models/delete_team_invitation_response.rs
- crates/tower-api/src/models/describe_run_logs_response.rs
- src/tower/tower_api_client/api/default/list_teams.py
- crates/tower-api/src/models/update_webhook_response.rs
- crates/tower-api/src/models/run_app_response.rs
- crates/tower-api/src/models/update_user_params.rs
- crates/tower-api/src/models/delete_api_key_params.rs
- crates/tower-api/src/models/pagination.rs
- crates/tower-api/src/models/list_guests_response.rs
- src/tower/tower_api_client/models/list_runs_status_item.py
- crates/tower-api/src/models/list_schedules_response.rs
- crates/tower-api/src/models/exported_secret.rs
- crates/tower-api/src/models/catalog_property.rs
- crates/tower-api/src/models/describe_webhook_response.rs
- crates/tower-api/src/apis/feature_flags_api.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/tower-cmd/src/mcp.rs
…#238) * fix: address review feedback on MCP parameter handling and run status - Enforce required fields (description, default) when adding a visible parameter via MCP; hidden parameters remain exempt - Prevent duplicate parameter names when renaming via edit_parameter - Handle Status::Failed explicitly in the local run monitor loop so polling stops instead of continuing indefinitely - Handle Status::Failed in the post-monitor result match with a clear error message rather than falling through to the generic error arm * fix: remove spurious description/default validation for visible parameters
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tower-api/src/models/invite_team_member_params.rs`:
- Around line 27-29: The role field in the InviteTeamMemberParams struct uses
#[serde(rename = "role", deserialize_with = "Option::deserialize")] which causes
missing role keys to fail deserialization; update the attribute list for the pub
role: Option<Role> field to include #[serde(default)] so absent keys deserialize
to None, and add #[serde(skip_serializing_if = "Option::is_none")] to avoid
emitting nulls on serialization; keep the existing rename and deserialize_with
attributes (role, Option::deserialize) intact so the field behavior remains
consistent.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ee92cdd-cb16-4c64-9181-7924d0e3ce58
📒 Files selected for processing (223)
crates/tower-api/README.mdcrates/tower-api/src/apis/configuration.rscrates/tower-api/src/apis/default_api.rscrates/tower-api/src/apis/feature_flags_api.rscrates/tower-api/src/models/account.rscrates/tower-api/src/models/acknowledge_alert_response.rscrates/tower-api/src/models/acknowledge_all_alerts_response.rscrates/tower-api/src/models/alert.rscrates/tower-api/src/models/api_key.rscrates/tower-api/src/models/app.rscrates/tower-api/src/models/app_statistics.rscrates/tower-api/src/models/app_summary.rscrates/tower-api/src/models/app_version.rscrates/tower-api/src/models/authentication_context.rscrates/tower-api/src/models/batch_schedule_params.rscrates/tower-api/src/models/batch_schedule_response.rscrates/tower-api/src/models/cancel_run_response.rscrates/tower-api/src/models/catalog.rscrates/tower-api/src/models/catalog_property.rscrates/tower-api/src/models/claim_device_login_ticket_params.rscrates/tower-api/src/models/claim_device_login_ticket_response.rscrates/tower-api/src/models/create_account_params.rscrates/tower-api/src/models/create_account_params_flags_struct.rscrates/tower-api/src/models/create_account_response.rscrates/tower-api/src/models/create_api_key_params.rscrates/tower-api/src/models/create_api_key_response.rscrates/tower-api/src/models/create_app_params.rscrates/tower-api/src/models/create_app_response.rscrates/tower-api/src/models/create_authenticator_params.rscrates/tower-api/src/models/create_authenticator_response.rscrates/tower-api/src/models/create_catalog_params.rscrates/tower-api/src/models/create_catalog_response.rscrates/tower-api/src/models/create_device_login_ticket_response.rscrates/tower-api/src/models/create_environment_params.rscrates/tower-api/src/models/create_environment_response.rscrates/tower-api/src/models/create_guest_params.rscrates/tower-api/src/models/create_guest_response.rscrates/tower-api/src/models/create_password_reset_params.rscrates/tower-api/src/models/create_password_reset_response.rscrates/tower-api/src/models/create_sandbox_secrets_params.rscrates/tower-api/src/models/create_sandbox_secrets_response.rscrates/tower-api/src/models/create_schedule_params.rscrates/tower-api/src/models/create_schedule_response.rscrates/tower-api/src/models/create_secret_params.rscrates/tower-api/src/models/create_secret_response.rscrates/tower-api/src/models/create_session_params.rscrates/tower-api/src/models/create_session_response.rscrates/tower-api/src/models/create_team_params.rscrates/tower-api/src/models/create_team_response.rscrates/tower-api/src/models/create_webhook_params.rscrates/tower-api/src/models/create_webhook_response.rscrates/tower-api/src/models/delete_api_key_params.rscrates/tower-api/src/models/delete_api_key_response.rscrates/tower-api/src/models/delete_app_response.rscrates/tower-api/src/models/delete_authenticator_params.rscrates/tower-api/src/models/delete_authenticator_response.rscrates/tower-api/src/models/delete_catalog_response.rscrates/tower-api/src/models/delete_guest_output_body.rscrates/tower-api/src/models/delete_schedule_params.rscrates/tower-api/src/models/delete_schedule_response.rscrates/tower-api/src/models/delete_secret_response.rscrates/tower-api/src/models/delete_session_params.rscrates/tower-api/src/models/delete_session_response.rscrates/tower-api/src/models/delete_team_invitation_params.rscrates/tower-api/src/models/delete_team_invitation_response.rscrates/tower-api/src/models/delete_team_params.rscrates/tower-api/src/models/delete_team_response.rscrates/tower-api/src/models/delete_webhook_response.rscrates/tower-api/src/models/deploy_app_request.rscrates/tower-api/src/models/deploy_app_response.rscrates/tower-api/src/models/describe_account_body.rscrates/tower-api/src/models/describe_app_response.rscrates/tower-api/src/models/describe_app_version_response.rscrates/tower-api/src/models/describe_authentication_context_body.rscrates/tower-api/src/models/describe_catalog_response.rscrates/tower-api/src/models/describe_device_login_session_response.rscrates/tower-api/src/models/describe_email_preferences_body.rscrates/tower-api/src/models/describe_plan_response.rscrates/tower-api/src/models/describe_run_graph_response.rscrates/tower-api/src/models/describe_run_links.rscrates/tower-api/src/models/describe_run_logs_response.rscrates/tower-api/src/models/describe_run_response.rscrates/tower-api/src/models/describe_secrets_key_response.rscrates/tower-api/src/models/describe_session_response.rscrates/tower-api/src/models/describe_team_response.rscrates/tower-api/src/models/describe_webhook_response.rscrates/tower-api/src/models/email_subscriptions.rscrates/tower-api/src/models/encrypted_catalog_property.rscrates/tower-api/src/models/environment.rscrates/tower-api/src/models/error_detail.rscrates/tower-api/src/models/error_model.rscrates/tower-api/src/models/event_alert.rscrates/tower-api/src/models/event_error.rscrates/tower-api/src/models/event_log.rscrates/tower-api/src/models/event_shouldertap.rscrates/tower-api/src/models/event_warning.rscrates/tower-api/src/models/export_catalogs_params.rscrates/tower-api/src/models/export_catalogs_response.rscrates/tower-api/src/models/export_secrets_params.rscrates/tower-api/src/models/export_secrets_response.rscrates/tower-api/src/models/exported_catalog.rscrates/tower-api/src/models/exported_catalog_property.rscrates/tower-api/src/models/exported_secret.rscrates/tower-api/src/models/feature.rscrates/tower-api/src/models/featurebase_identity.rscrates/tower-api/src/models/generate_app_statistics_response.rscrates/tower-api/src/models/generate_authenticator_response.rscrates/tower-api/src/models/generate_run_statistics_response.rscrates/tower-api/src/models/generate_runner_credentials_response.rscrates/tower-api/src/models/get_feature_flag_response_body.rscrates/tower-api/src/models/guest.rscrates/tower-api/src/models/invite_team_member_params.rscrates/tower-api/src/models/invite_team_member_response.rscrates/tower-api/src/models/leave_team_response.rscrates/tower-api/src/models/list_alerts_response.rscrates/tower-api/src/models/list_api_keys_response.rscrates/tower-api/src/models/list_app_environments_response.rscrates/tower-api/src/models/list_app_versions_response.rscrates/tower-api/src/models/list_apps_response.rscrates/tower-api/src/models/list_authenticators_response.rscrates/tower-api/src/models/list_catalogs_response.rscrates/tower-api/src/models/list_environments_response.rscrates/tower-api/src/models/list_guests_response.rscrates/tower-api/src/models/list_my_team_invitations_response.rscrates/tower-api/src/models/list_runners_response.rscrates/tower-api/src/models/list_runs_response.rscrates/tower-api/src/models/list_schedules_response.rscrates/tower-api/src/models/list_secret_environments_response.rscrates/tower-api/src/models/list_secrets_response.rscrates/tower-api/src/models/list_team_invitations_response.rscrates/tower-api/src/models/list_team_members_response.rscrates/tower-api/src/models/list_teams_response.rscrates/tower-api/src/models/list_webhooks_response.rscrates/tower-api/src/models/mod.rscrates/tower-api/src/models/organization.rscrates/tower-api/src/models/organization_usage.rscrates/tower-api/src/models/pagination.rscrates/tower-api/src/models/parameter.rscrates/tower-api/src/models/plan.rscrates/tower-api/src/models/refresh_session_params.rscrates/tower-api/src/models/refresh_session_response.rscrates/tower-api/src/models/regenerate_guest_login_url_params.rscrates/tower-api/src/models/regenerate_guest_login_url_response.rscrates/tower-api/src/models/remove_team_member_params.rscrates/tower-api/src/models/remove_team_member_response.rscrates/tower-api/src/models/resend_team_invitation_params.rscrates/tower-api/src/models/resend_team_invitation_response.rscrates/tower-api/src/models/run.rscrates/tower-api/src/models/run_app_initiator_data.rscrates/tower-api/src/models/run_app_params.rscrates/tower-api/src/models/run_app_response.rscrates/tower-api/src/models/run_attempt.rscrates/tower-api/src/models/run_failure_alert.rscrates/tower-api/src/models/run_graph_node.rscrates/tower-api/src/models/run_graph_run_id.rscrates/tower-api/src/models/run_initiator.rscrates/tower-api/src/models/run_initiator_details.rscrates/tower-api/src/models/run_log_line.rscrates/tower-api/src/models/run_parameter.rscrates/tower-api/src/models/run_results.rscrates/tower-api/src/models/run_retry_policy.rscrates/tower-api/src/models/run_run_initiator_details.rscrates/tower-api/src/models/run_statistics.rscrates/tower-api/src/models/run_timeseries_point.rscrates/tower-api/src/models/runner.rscrates/tower-api/src/models/runner_credentials.rscrates/tower-api/src/models/schedule.rscrates/tower-api/src/models/schedule_run_initiator_details.rscrates/tower-api/src/models/search_runs_response.rscrates/tower-api/src/models/secret.rscrates/tower-api/src/models/server_sent_events_inner.rscrates/tower-api/src/models/server_sent_events_inner_1.rscrates/tower-api/src/models/server_sent_events_inner_2.rscrates/tower-api/src/models/session.rscrates/tower-api/src/models/shoulder_tap.rscrates/tower-api/src/models/sse_warning.rscrates/tower-api/src/models/statistics_settings.rscrates/tower-api/src/models/team.rscrates/tower-api/src/models/team_invitation.rscrates/tower-api/src/models/team_membership.rscrates/tower-api/src/models/test_webhook_response.rscrates/tower-api/src/models/token.rscrates/tower-api/src/models/unverified_authenticator.rscrates/tower-api/src/models/update_account_params.rscrates/tower-api/src/models/update_account_response.rscrates/tower-api/src/models/update_app_params.rscrates/tower-api/src/models/update_app_response.rscrates/tower-api/src/models/update_catalog_params.rscrates/tower-api/src/models/update_catalog_response.rscrates/tower-api/src/models/update_email_preferences_body.rscrates/tower-api/src/models/update_environment_params.rscrates/tower-api/src/models/update_environment_response.rscrates/tower-api/src/models/update_my_team_invitation_params.rscrates/tower-api/src/models/update_my_team_invitation_response.rscrates/tower-api/src/models/update_organization_params.rscrates/tower-api/src/models/update_organization_response.rscrates/tower-api/src/models/update_password_reset_params.rscrates/tower-api/src/models/update_password_reset_response.rscrates/tower-api/src/models/update_plan_params.rscrates/tower-api/src/models/update_plan_response.rscrates/tower-api/src/models/update_schedule_params.rscrates/tower-api/src/models/update_schedule_response.rscrates/tower-api/src/models/update_secret_params.rscrates/tower-api/src/models/update_secret_response.rscrates/tower-api/src/models/update_team_member_params.rscrates/tower-api/src/models/update_team_member_response.rscrates/tower-api/src/models/update_team_params.rscrates/tower-api/src/models/update_team_response.rscrates/tower-api/src/models/update_user_params.rscrates/tower-api/src/models/update_user_response.rscrates/tower-api/src/models/update_webhook_params.rscrates/tower-api/src/models/update_webhook_response.rscrates/tower-api/src/models/usage_limit.rscrates/tower-api/src/models/user.rscrates/tower-api/src/models/verified_authenticator.rscrates/tower-api/src/models/verify_email_params.rscrates/tower-api/src/models/verify_email_response.rscrates/tower-api/src/models/webhook.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/run.rssrc/tower/tower_api_client/api/default/describe_catalog.pysrc/tower/tower_api_client/models/__init__.pysrc/tower/tower_api_client/models/describe_catalog_response.py
✅ Files skipped from review due to trivial changes (178)
- crates/tower-api/src/models/acknowledge_alert_response.rs
- crates/tower-api/src/models/app_version.rs
- crates/tower-api/src/models/batch_schedule_params.rs
- crates/tower-api/src/models/cancel_run_response.rs
- crates/tower-api/src/models/catalog_property.rs
- crates/tower-api/src/models/claim_device_login_ticket_response.rs
- crates/tower-api/src/models/create_account_params.rs
- crates/tower-api/src/models/create_account_response.rs
- crates/tower-api/src/models/create_api_key_response.rs
- crates/tower-api/src/models/create_app_params.rs
- crates/tower-api/src/models/create_app_response.rs
- crates/tower-api/src/models/create_authenticator_params.rs
- crates/tower-api/src/models/create_catalog_params.rs
- crates/tower-api/src/models/create_catalog_response.rs
- crates/tower-api/src/models/create_environment_params.rs
- crates/tower-api/src/models/create_environment_response.rs
- crates/tower-api/src/models/create_password_reset_params.rs
- crates/tower-api/src/models/create_password_reset_response.rs
- crates/tower-api/src/models/create_sandbox_secrets_response.rs
- crates/tower-api/src/models/create_schedule_response.rs
- crates/tower-api/src/models/create_secret_params.rs
- crates/tower-api/src/models/create_secret_response.rs
- crates/tower-api/src/models/create_session_params.rs
- crates/tower-api/src/models/create_session_response.rs
- crates/tower-api/src/models/create_webhook_params.rs
- crates/tower-api/src/models/create_webhook_response.rs
- crates/tower-api/src/models/delete_api_key_params.rs
- crates/tower-api/src/models/delete_api_key_response.rs
- crates/tower-api/src/models/delete_app_response.rs
- crates/tower-api/src/models/delete_authenticator_params.rs
- crates/tower-api/src/models/delete_authenticator_response.rs
- crates/tower-api/src/models/delete_catalog_response.rs
- crates/tower-api/src/models/delete_schedule_params.rs
- crates/tower-api/src/models/delete_schedule_response.rs
- crates/tower-api/src/models/create_team_params.rs
- crates/tower-api/src/models/delete_secret_response.rs
- crates/tower-api/src/models/delete_session_params.rs
- crates/tower-api/src/models/delete_session_response.rs
- crates/tower-api/src/models/delete_team_invitation_response.rs
- crates/tower-api/src/models/delete_team_params.rs
- crates/tower-api/src/models/delete_webhook_response.rs
- crates/tower-api/src/models/deploy_app_request.rs
- crates/tower-api/src/models/describe_account_body.rs
- crates/tower-api/src/models/describe_app_response.rs
- crates/tower-api/src/models/describe_app_version_response.rs
- crates/tower-api/src/models/describe_authentication_context_body.rs
- crates/tower-api/src/models/describe_email_preferences_body.rs
- crates/tower-api/src/models/describe_plan_response.rs
- crates/tower-api/src/models/describe_run_logs_response.rs
- crates/tower-api/src/models/describe_secrets_key_response.rs
- crates/tower-api/src/models/describe_session_response.rs
- crates/tower-api/src/models/describe_team_response.rs
- crates/tower-api/src/models/describe_webhook_response.rs
- crates/tower-api/src/models/email_subscriptions.rs
- crates/tower-api/src/models/error_model.rs
- crates/tower-api/src/models/event_alert.rs
- crates/tower-api/src/models/event_log.rs
- crates/tower-api/src/models/event_error.rs
- crates/tower-api/src/models/export_catalogs_params.rs
- crates/tower-api/src/models/export_catalogs_response.rs
- crates/tower-api/src/models/export_secrets_params.rs
- crates/tower-api/src/models/export_secrets_response.rs
- crates/tower-api/src/models/exported_secret.rs
- crates/tower-api/src/models/feature.rs
- crates/tower-api/src/models/featurebase_identity.rs
- crates/tower-api/src/models/generate_app_statistics_response.rs
- crates/tower-api/src/models/generate_authenticator_response.rs
- crates/tower-api/src/models/generate_run_statistics_response.rs
- crates/tower-api/src/models/generate_runner_credentials_response.rs
- crates/tower-api/src/models/get_feature_flag_response_body.rs
- crates/tower-api/src/models/list_alerts_response.rs
- crates/tower-api/src/models/list_api_keys_response.rs
- crates/tower-api/src/models/list_app_environments_response.rs
- crates/tower-api/src/models/list_app_versions_response.rs
- crates/tower-api/src/models/list_apps_response.rs
- crates/tower-api/src/models/list_environments_response.rs
- crates/tower-api/src/models/list_schedules_response.rs
- crates/tower-api/src/apis/configuration.rs
- crates/tower-api/src/models/acknowledge_all_alerts_response.rs
- crates/tower-api/src/models/alert.rs
- crates/tower-api/src/models/app_statistics.rs
- crates/tower-api/src/models/app_summary.rs
- crates/tower-api/src/models/authentication_context.rs
- crates/tower-api/src/models/batch_schedule_response.rs
- crates/tower-api/src/models/catalog.rs
- crates/tower-api/src/models/claim_device_login_ticket_params.rs
- crates/tower-api/src/models/create_account_params_flags_struct.rs
- crates/tower-api/src/models/create_sandbox_secrets_params.rs
- crates/tower-api/src/models/delete_guest_output_body.rs
- crates/tower-api/src/models/describe_device_login_session_response.rs
- crates/tower-api/src/models/describe_run_response.rs
- crates/tower-api/src/models/encrypted_catalog_property.rs
- crates/tower-api/src/models/list_catalogs_response.rs
- crates/tower-api/src/models/list_runs_response.rs
- crates/tower-api/src/models/list_my_team_invitations_response.rs
- crates/tower-api/src/models/list_secret_environments_response.rs
- crates/tower-api/src/models/list_webhooks_response.rs
- crates/tower-api/src/models/list_secrets_response.rs
- crates/tower-api/src/models/event_warning.rs
- crates/tower-api/src/models/list_team_invitations_response.rs
- crates/tower-api/src/models/leave_team_response.rs
- crates/tower-api/src/models/resend_team_invitation_response.rs
- crates/tower-api/src/models/pagination.rs
- crates/tower-api/src/models/delete_team_invitation_params.rs
- crates/tower-api/src/models/refresh_session_response.rs
- crates/tower-api/src/models/list_teams_response.rs
- crates/tower-api/src/models/organization.rs
- crates/tower-api/src/models/token.rs
- crates/tower-api/src/models/update_account_response.rs
- crates/tower-api/src/models/update_my_team_invitation_params.rs
- crates/tower-api/src/models/update_catalog_response.rs
- crates/tower-api/src/models/update_my_team_invitation_response.rs
- crates/tower-api/src/models/update_catalog_params.rs
- crates/tower-api/src/models/update_user_response.rs
- crates/tower-api/src/models/update_plan_response.rs
- crates/tower-api/src/models/update_webhook_params.rs
- crates/tower-api/src/models/update_environment_response.rs
- crates/tower-api/src/models/update_organization_response.rs
- crates/tower-api/src/models/run_app_initiator_data.rs
- crates/tower-api/src/models/run_graph_node.rs
- crates/tower-api/src/models/list_guests_response.rs
- crates/tower-api/src/models/guest.rs
- crates/tower-api/src/models/plan.rs
- crates/tower-api/src/models/list_runners_response.rs
- crates/tower-api/src/models/update_user_params.rs
- crates/tower-api/src/models/run_app_response.rs
- crates/tower-api/src/models/run_initiator_details.rs
- crates/tower-api/src/models/list_authenticators_response.rs
- crates/tower-api/src/models/run_initiator.rs
- crates/tower-api/src/models/remove_team_member_params.rs
- crates/tower-api/src/models/run_statistics.rs
- crates/tower-api/src/models/run_failure_alert.rs
- crates/tower-api/src/models/remove_team_member_response.rs
- crates/tower-api/src/models/run_run_initiator_details.rs
- crates/tower-api/src/models/list_team_members_response.rs
- crates/tower-api/src/models/event_shouldertap.rs
- crates/tower-api/src/models/schedule_run_initiator_details.rs
- crates/tower-api/src/models/secret.rs
- crates/tower-api/src/models/search_runs_response.rs
- crates/tower-api/src/models/session.rs
- crates/tower-api/src/models/server_sent_events_inner.rs
- crates/tower-api/src/models/server_sent_events_inner_1.rs
- crates/tower-api/src/models/statistics_settings.rs
- crates/tower-api/src/models/sse_warning.rs
- crates/tower-api/src/models/create_team_response.rs
- crates/tower-api/src/models/invite_team_member_response.rs
- crates/tower-api/src/models/shoulder_tap.rs
- crates/tower-api/src/models/team_membership.rs
- crates/tower-api/src/models/test_webhook_response.rs
- crates/tower-api/src/models/unverified_authenticator.rs
- crates/tower-api/src/models/update_organization_params.rs
- crates/tower-api/src/models/update_app_response.rs
- crates/tower-api/src/models/update_schedule_response.rs
- crates/tower-api/src/models/update_secret_params.rs
- crates/tower-api/src/models/update_webhook_response.rs
- crates/tower-api/src/models/update_environment_params.rs
- crates/tower-api/src/models/update_secret_response.rs
- crates/tower-api/src/models/update_team_member_params.rs
- crates/tower-api/src/models/verify_email_params.rs
- crates/tower-api/src/models/webhook.rs
- crates/tower-api/src/models/user.rs
- crates/tower-api/src/models/update_plan_params.rs
- crates/tower-api/src/models/update_team_response.rs
- crates/tower-api/src/models/update_team_member_response.rs
- crates/tower-api/src/models/verified_authenticator.rs
- crates/tower-api/src/models/verify_email_response.rs
- crates/tower-api/src/models/exported_catalog.rs
- crates/tower-api/src/models/update_password_reset_response.rs
- crates/tower-api/src/models/deploy_app_response.rs
- crates/tower-api/src/models/environment.rs
- crates/tower-api/src/models/error_detail.rs
- crates/tower-api/src/apis/feature_flags_api.rs
- crates/tower-api/src/models/usage_limit.rs
- crates/tower-api/src/models/exported_catalog_property.rs
- src/tower/tower_api_client/models/init.py
- crates/tower-api/src/models/organization_usage.rs
- crates/tower-api/src/models/refresh_session_params.rs
- crates/tower-api/src/models/update_password_reset_params.rs
🚧 Files skipped from review as they are similar to previous changes (31)
- crates/tower-api/src/models/create_authenticator_response.rs
- crates/tower-api/src/models/create_device_login_ticket_response.rs
- crates/tower-api/src/models/delete_team_response.rs
- crates/tower-api/src/models/describe_run_graph_response.rs
- crates/tower-api/src/models/account.rs
- crates/tower-api/src/models/api_key.rs
- crates/tower-api/src/models/app.rs
- crates/tower-api/src/models/create_api_key_params.rs
- crates/tower-api/src/models/create_guest_response.rs
- crates/tower-api/src/models/create_schedule_params.rs
- crates/tower-api/src/models/regenerate_guest_login_url_params.rs
- crates/tower-api/src/models/create_guest_params.rs
- crates/tower-api/src/models/update_email_preferences_body.rs
- crates/tower-api/src/models/run_app_params.rs
- crates/tower-api/src/models/regenerate_guest_login_url_response.rs
- crates/tower-api/src/models/server_sent_events_inner_2.rs
- crates/tower-api/src/models/runner_credentials.rs
- crates/tower-api/src/models/run_graph_run_id.rs
- crates/tower-api/src/models/update_app_params.rs
- crates/tower-api/src/models/resend_team_invitation_params.rs
- crates/tower-api/src/models/run_timeseries_point.rs
- crates/tower-api/src/models/run_parameter.rs
- crates/tower-api/src/models/schedule.rs
- crates/tower-api/src/models/run_retry_policy.rs
- crates/tower-api/README.md
- crates/tower-api/src/models/update_account_params.rs
- crates/tower-api/src/models/run_results.rs
- crates/tower-api/src/models/team_invitation.rs
- crates/tower-api/src/models/team.rs
- crates/tower-cmd/src/mcp.rs
- crates/tower-api/src/models/describe_run_links.rs
Comment on lines
+27
to
+29
| /// The role to assign to the invitations | ||
| #[serde(rename = "role", deserialize_with = "Option::deserialize")] | ||
| pub role: Option<Role>, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Rust serde derive, for a struct field role: Optionwith#[serde(deserialize_with = "Option::deserialize")], does a missing JSON field deserialize to Noneautomatically, or is#[serde(default)] required to avoid a missing-field error?
💡 Result:
No, a missing JSON field does not deserialize to None automatically when using #[serde(deserialize_with = "Option::deserialize")]. It will fail with a "missing field" error. The #[serde(default)] attribute is required to handle missing fields by deserializing them to None. Without deserialize_with, an Option field deserializes missing fields to None by default. However, adding #[serde(deserialize_with = "Option::deserialize")] overrides this special handling. The custom deserializer is only invoked if the field is present in the JSON. If the field is missing, Serde treats it as required and errors out, because the Option type no longer signals optionality to the derive macro. To restore the missing-field-to-None behavior, combine it with #[serde(default)]: #[serde(default, deserialize_with = "Option::deserialize")] role: Option, This uses the type's Default impl (None for Option) when the field is missing, and calls the custom deserializer when present. This behavior is confirmed across multiple sources including official Serde GitHub issues and Stack Overflow answers with code examples.
Citations:
- 1:
deserialize_withcallback doesn't get called on Option<T> values, fails with an error "missing field" serde-rs/serde#1951 - 2: Serde behavior for some deserializers with Option and missing fields is counterintuitive serde-rs/serde#2214
- 3: https://stackoverflow.com/questions/68080171/deserializer-never-called-when-field-is-not-present
- 4: https://stackoverflow.com/questions/44301748/how-can-i-deserialize-an-optional-field-with-custom-functions-using-serde
- 5: https://stackoverflow.com/questions/65164276/lifting-nested-values-from-json-that-might-be-optional-using-serde-in-rust
- 6: https://serde.rs/field-attrs.html
Add #[serde(default)] to prevent deserialization errors on missing role field
When deserialize_with is used on an Option<T> field, the custom deserializer is only invoked if the field is present in JSON. If the field is missing, Serde treats it as required and fails with a missing-field error. The default attribute restores the correct behavior, allowing missing fields to deserialize to None. Additionally, add skip_serializing_if to avoid serializing unset values as null, which can break API compatibility for endpoints expecting omitted optional keys.
Suggested fix
- #[serde(rename = "role", deserialize_with = "Option::deserialize")]
+ #[serde(
+ rename = "role",
+ default,
+ skip_serializing_if = "Option::is_none",
+ deserialize_with = "Option::deserialize"
+ )]
pub role: Option<Role>,📝 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
| /// The role to assign to the invitations | |
| #[serde(rename = "role", deserialize_with = "Option::deserialize")] | |
| pub role: Option<Role>, | |
| /// The role to assign to the invitations | |
| #[serde( | |
| rename = "role", | |
| default, | |
| skip_serializing_if = "Option::is_none", | |
| deserialize_with = "Option::deserialize" | |
| )] | |
| pub role: Option<Role>, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tower-api/src/models/invite_team_member_params.rs` around lines 27 -
29, The role field in the InviteTeamMemberParams struct uses #[serde(rename =
"role", deserialize_with = "Option::deserialize")] which causes missing role
keys to fail deserialization; update the attribute list for the pub role:
Option<Role> field to include #[serde(default)] so absent keys deserialize to
None, and add #[serde(skip_serializing_if = "Option::is_none")] to avoid
emitting nulls on serialization; keep the existing rename and deserialize_with
attributes (role, Option::deserialize) intact so the field behavior remains
consistent.
* feat: add TOWER_API_KEY support to Config Add `api_key` field to Config, populated from the TOWER_API_KEY env var. When set, make_api_configuration() uses the X-API-Key header instead of Bearer JWT auth, skipping session-based authentication entirely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: skip session creation when TOWER_API_KEY is set When TOWER_API_KEY env var is present, skip JWT validation and session.json loading entirely. The API key is self-contained and doesn't need a persisted session. Priority order: TOWER_API_KEY > TOWER_JWT > session.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat: warn during tower login when TOWER_API_KEY is set Show a yellow warning explaining that the API key will continue to take precedence over the session created by the login flow. Prompt the user to confirm before proceeding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add integration tests for API key authentication Add cli_api_key_auth.feature with scenarios for: - Listing apps with API key (verifies JSON output, no session.json created) - Listing teams with API key (verifies no session.json created) - Login warning when TOWER_API_KEY is set Add "via CLI with API key" step that uses TOWER_API_KEY env var with a temp HOME (no session.json) to prove API key auth works standalone. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: support tower teams list with API key auth When authenticating via API key, call the /teams API endpoint directly instead of refreshing the session (which requires a session.json). Add list_teams API wrapper and ResponseEntity impl for ListTeamsSuccess. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: support MCP tower_teams_list with API key auth When authenticating via API key, call the /teams endpoint directly instead of refreshing the session. Mirrors the same fix applied to the CLI teams list command. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix: formatting --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/tower-cmd/src/lib.rs (1)
33-42:⚠️ Potential issue | 🟠 MajorClearing
sessionhere regresses commands that still require it.
crates/tower-cmd/src/run.rs, Line 183-187still doesconfig.session.as_ref().ok_or(Error::NoSession)?, and the MCP session-backed flow aroundcrates/tower-cmd/src/mcp.rs, Line 671-685still needs a current session. With this branch returningNone, those commands now fail underTOWER_API_KEYbefore they ever reach the new header-based auth path.Either migrate those callers in this PR or keep the current session-loading behavior until the direct
config.sessionconsumers are gone.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tower-cmd/src/lib.rs` around lines 33 - 42, The current early-return to None when TOWER_API_KEY is set breaks callers that still need a Session (e.g., run.rs uses config.session.as_ref().ok_or and mcp.rs relies on a session-backed flow). Remove the branch that forces session = None when TOWER_API_KEY is present; instead always attempt to load a Session (prefer Session::from_jwt if TOWER_JWT exists, else Session::from_config_dir), and keep using the API key only for header-based auth elsewhere — this preserves the Session value used by run.rs and mcp.rs while allowing API-key header auth to coexist.crates/config/src/lib.rs (1)
191-207:⚠️ Potential issue | 🟠 Major
make_api_configuration()still ignores the injected session.After the API-key early return, the bearer-token branch only re-reads
session.json. Any session passed in viawith_session()—notably theTOWER_JWTpath inApp::new()—never reaches API clients, so JWT-only invocations still fall back to disk or end up unauthenticated.Please preserve the injected session here, or track whether it came from env vs. disk before applying the “read from disk” fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/config/src/lib.rs` around lines 191 - 207, The make_api_configuration() function currently returns early for api_key but then always re-reads Session::from_config_dir(), which overrides any session injected via with_session() (used by App::new() / TOWER_JWT); change the logic to prefer an already-injected self.session (or a flag indicating it came from env) before falling back to Session::from_config_dir(): if self.session.is_some() use that session's token to set configuration.bearer_access_token, otherwise attempt Session::from_config_dir() as the disk fallback; ensure you do not overwrite configuration.bearer_access_token when an injected session is present.
♻️ Duplicate comments (1)
crates/tower-cmd/src/mcp.rs (1)
845-865:⚠️ Potential issue | 🟠 MajorVisible parameters can still be written with missing metadata.
addstill defaults omitteddescriptionanddefaultto empty strings, andeditcan turn a hidden parameter visible without re-validating that the mergeddescriptionanddefaultare present. That leaves the Towerfile in a state the tool descriptions say is invalid.Proposed fix
async fn tower_file_add_parameter( &self, Parameters(request): Parameters<AddParameterRequest>, ) -> Result<CallToolResult, McpError> { if request.hidden && request.default.is_some() { return Self::text_error("hidden and default are mutually exclusive".into()); } + if !request.hidden { + if request + .description + .as_deref() + .map(str::trim) + .unwrap_or("") + .is_empty() + { + return Self::text_error("description is required when hidden=false".into()); + } + if request + .default + .as_deref() + .map(str::trim) + .unwrap_or("") + .is_empty() + { + return Self::text_error("default is required when hidden=false".into()); + } + } let name = request.name.clone(); Self::modify_towerfile(&request.common, |tf| { tf.set_parameter(&name, Parameter { name: name.clone(), description: request.description.unwrap_or_default(), @@ let param = Parameter { name: target_name, description: request.description.unwrap_or_else(|| existing.description.clone()), default: request.default.unwrap_or_else(|| existing.default.clone()), hidden: request.hidden.unwrap_or(existing.hidden), }; + if !param.hidden + && (param.description.trim().is_empty() || param.default.trim().is_empty()) + { + return Err("description and default are required when hidden=false".into()); + } if param.hidden && !param.default.is_empty() { return Err("hidden and default are mutually exclusive".into()); }Also applies to: 867-897
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/tower-cmd/src/mcp.rs` around lines 845 - 865, The add/edit handlers (tower_file_add_parameter and the corresponding edit path that calls modify_towerfile/set_parameter) currently default missing description/default to empty strings and allow turning a hidden param visible without validating merged metadata; change the logic so that when creating or updating a parameter you do not silently fill missing description/default: if request.hidden is false (or the resulting merged parameter.hidden is false on edit) then require non-empty description and default and return a text error if either is missing; update the closure passed to modify_towerfile (and the merge logic used by tower_file_edit_parameter) to validate the combined/merged Parameter fields before calling tf.set_parameter and reject the change with an explicit error instead of writing empty strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tower-cmd/src/mcp.rs`:
- Around line 982-983: Replace the ambiguous shorthand mentions of
parameter-modifying tools with their exact MCP tool names: keep
tower_file_generate and tower_file_update as-is but change any occurrences of
"tower_file_add/edit/remove_parameter" (and the earlier shorthand at the other
location) to the explicit names tower_file_add_parameter,
tower_file_edit_parameter, and tower_file_remove_parameter so the guidance text
only references valid tool identifiers.
- Around line 856-863: The add path (tower_file_add_parameter) currently uses
tf.set_parameter (an upsert) and can overwrite existing parameters; change
modify_towerfile closure to first check whether a parameter with the given name
already exists (e.g., via tf.get_parameter or tf.parameters lookup) and return
an Err or appropriate failure if it does, only calling tf.set_parameter when no
existing parameter is found so add fails fast instead of clobbering metadata
(leave tower_file_edit_parameter to perform updates).
In `@tests/integration/features/steps/cli_steps.py`:
- Around line 69-84: The subprocess environment used for API-key tests currently
starts from os.environ.copy() which can leak parent auth (e.g., TOWER_JWT) into
the child; change the runner to create a minimal environment (e.g., start from
an empty dict or os.environ.copy() and then explicitly remove auth-related keys)
before setting FORCE_COLOR, CLICOLOR_FORCE, TOWER_URL, TOWER_API_KEY and HOME so
that the subprocess run call (subprocess.run invoked with full_command and
env=test_env) executes with only API-key auth present; update test_env handling
where it is built to ensure any keys like TOWER_JWT, TOWER_SESSION, or similar
auth overrides are not present.
---
Outside diff comments:
In `@crates/config/src/lib.rs`:
- Around line 191-207: The make_api_configuration() function currently returns
early for api_key but then always re-reads Session::from_config_dir(), which
overrides any session injected via with_session() (used by App::new() /
TOWER_JWT); change the logic to prefer an already-injected self.session (or a
flag indicating it came from env) before falling back to
Session::from_config_dir(): if self.session.is_some() use that session's token
to set configuration.bearer_access_token, otherwise attempt
Session::from_config_dir() as the disk fallback; ensure you do not overwrite
configuration.bearer_access_token when an injected session is present.
In `@crates/tower-cmd/src/lib.rs`:
- Around line 33-42: The current early-return to None when TOWER_API_KEY is set
breaks callers that still need a Session (e.g., run.rs uses
config.session.as_ref().ok_or and mcp.rs relies on a session-backed flow).
Remove the branch that forces session = None when TOWER_API_KEY is present;
instead always attempt to load a Session (prefer Session::from_jwt if TOWER_JWT
exists, else Session::from_config_dir), and keep using the API key only for
header-based auth elsewhere — this preserves the Session value used by run.rs
and mcp.rs while allowing API-key header auth to coexist.
---
Duplicate comments:
In `@crates/tower-cmd/src/mcp.rs`:
- Around line 845-865: The add/edit handlers (tower_file_add_parameter and the
corresponding edit path that calls modify_towerfile/set_parameter) currently
default missing description/default to empty strings and allow turning a hidden
param visible without validating merged metadata; change the logic so that when
creating or updating a parameter you do not silently fill missing
description/default: if request.hidden is false (or the resulting merged
parameter.hidden is false on edit) then require non-empty description and
default and return a text error if either is missing; update the closure passed
to modify_towerfile (and the merge logic used by tower_file_edit_parameter) to
validate the combined/merged Parameter fields before calling tf.set_parameter
and reject the change with an explicit error instead of writing empty strings.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed2cd686-552f-4673-91a5-7de564155a6b
📒 Files selected for processing (8)
crates/config/src/lib.rscrates/tower-cmd/src/api.rscrates/tower-cmd/src/lib.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/session.rscrates/tower-cmd/src/teams.rstests/integration/features/cli_api_key_auth.featuretests/integration/features/steps/cli_steps.py
✅ Files skipped from review due to trivial changes (1)
- tests/integration/features/cli_api_key_auth.feature
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/tower-cmd/src/api.rs
Comment on lines
+856
to
+863
| Self::modify_towerfile(&request.common, |tf| { | ||
| tf.set_parameter(&name, Parameter { | ||
| name: name.clone(), | ||
| description: request.description.unwrap_or_default(), | ||
| default: request.default.unwrap_or_default(), | ||
| hidden: request.hidden, | ||
| }); | ||
| Ok(format!("Added parameter '{name}'")) |
There was a problem hiding this comment.
tower_file_add_parameter shouldn't silently overwrite existing entries.
tf.set_parameter() is an upsert. Now that tower_file_edit_parameter exists, a mistaken add call can clobber an existing parameter's metadata instead of failing fast.
Proposed fix
let name = request.name.clone();
Self::modify_towerfile(&request.common, |tf| {
+ if tf.parameters.iter().any(|p| p.name == name) {
+ return Err(format!(
+ "Parameter '{}' already exists; use tower_file_edit_parameter",
+ name
+ ));
+ }
tf.set_parameter(&name, Parameter {
name: name.clone(),
description: request.description.unwrap_or_default(),
default: request.default.unwrap_or_default(),
hidden: request.hidden,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tower-cmd/src/mcp.rs` around lines 856 - 863, The add path
(tower_file_add_parameter) currently uses tf.set_parameter (an upsert) and can
overwrite existing parameters; change modify_towerfile closure to first check
whether a parameter with the given name already exists (e.g., via
tf.get_parameter or tf.parameters lookup) and return an Err or appropriate
failure if it does, only calling tf.set_parameter when no existing parameter is
found so add fails fast instead of clobbering metadata (leave
tower_file_edit_parameter to perform updates).
Comment on lines
+982
to
+983
| tower_file_generate → tower_file_update → tower_file_add/edit/remove_parameter → tower_file_validate | ||
| CRITICAL: Always use tower_file_update or tower_file_add/edit/remove_parameter to modify |
There was a problem hiding this comment.
Use exact MCP tool names in the guidance text.
The current tower_file_update/add/edit/remove_parameter shorthand implies tools like tower_file_add or tower_file_edit, which don't exist. Because the model learns from these strings, this can steer it into invalid tool calls. Spell out tower_file_add_parameter, tower_file_edit_parameter, and tower_file_remove_parameter explicitly in both places.
Also applies to: 1118-1121
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tower-cmd/src/mcp.rs` around lines 982 - 983, Replace the ambiguous
shorthand mentions of parameter-modifying tools with their exact MCP tool names:
keep tower_file_generate and tower_file_update as-is but change any occurrences
of "tower_file_add/edit/remove_parameter" (and the earlier shorthand at the
other location) to the explicit names tower_file_add_parameter,
tower_file_edit_parameter, and tower_file_remove_parameter so the guidance text
only references valid tool identifiers.
Comment on lines
+69
to
+84
| test_env = os.environ.copy() | ||
| test_env["FORCE_COLOR"] = "1" | ||
| test_env["CLICOLOR_FORCE"] = "1" | ||
| test_env["TOWER_URL"] = context.tower_url | ||
| test_env["TOWER_API_KEY"] = "sk-test-api-key" | ||
|
|
||
| # Use a temp HOME with no session.json to prove API key auth works standalone | ||
| test_env["HOME"] = context.temp_dir | ||
|
|
||
| result = subprocess.run( | ||
| full_command, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=60, | ||
| env=test_env, | ||
| ) |
There was a problem hiding this comment.
Drop inherited auth overrides in the API-key runner.
This helper starts from os.environ.copy(), so any parent TOWER_JWT or similar auth override leaks into the subprocess. That makes these scenarios sensitive to mixed-auth environments instead of exercising a clean API-key-only path.
Proposed fix
test_env["FORCE_COLOR"] = "1"
test_env["CLICOLOR_FORCE"] = "1"
test_env["TOWER_URL"] = context.tower_url
+ test_env.pop("TOWER_JWT", None)
test_env["TOWER_API_KEY"] = "sk-test-api-key"📝 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
| test_env = os.environ.copy() | |
| test_env["FORCE_COLOR"] = "1" | |
| test_env["CLICOLOR_FORCE"] = "1" | |
| test_env["TOWER_URL"] = context.tower_url | |
| test_env["TOWER_API_KEY"] = "sk-test-api-key" | |
| # Use a temp HOME with no session.json to prove API key auth works standalone | |
| test_env["HOME"] = context.temp_dir | |
| result = subprocess.run( | |
| full_command, | |
| capture_output=True, | |
| text=True, | |
| timeout=60, | |
| env=test_env, | |
| ) | |
| test_env = os.environ.copy() | |
| test_env["FORCE_COLOR"] = "1" | |
| test_env["CLICOLOR_FORCE"] = "1" | |
| test_env["TOWER_URL"] = context.tower_url | |
| test_env.pop("TOWER_JWT", None) | |
| test_env["TOWER_API_KEY"] = "sk-test-api-key" | |
| # Use a temp HOME with no session.json to prove API key auth works standalone | |
| test_env["HOME"] = context.temp_dir | |
| result = subprocess.run( | |
| full_command, | |
| capture_output=True, | |
| text=True, | |
| timeout=60, | |
| env=test_env, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration/features/steps/cli_steps.py` around lines 69 - 84, The
subprocess environment used for API-key tests currently starts from
os.environ.copy() which can leak parent auth (e.g., TOWER_JWT) into the child;
change the runner to create a minimal environment (e.g., start from an empty
dict or os.environ.copy() and then explicitly remove auth-related keys) before
setting FORCE_COLOR, CLICOLOR_FORCE, TOWER_URL, TOWER_API_KEY and HOME so that
the subprocess run call (subprocess.run invoked with full_command and
env=test_env) executes with only API-key auth present; update test_env handling
where it is built to ensure any keys like TOWER_JWT, TOWER_SESSION, or similar
auth overrides are not present.
* feat: introduce catalog cli commands * consolidate header style * fix cli_steps
Merged
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
5 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