Conversation
The legacy-setuptools retry only fired for requirements.txt projects, leaving pyproject.toml apps with no recovery path when their builds needed pkg_resources. The retry now drops out of uv sync into uv pip install with the project source (. for pyproject, -r requirements.txt otherwise) plus a setuptools<82 arg, so both kinds of project get the same fallback.
Removes the should_use_legacy_setuptools_pin predicate and the spawn_requirements_install helper — the call site in tower-runtime is now just 'if res != 0 { retry }', and the file-type branch lives inline where the args are built.
* chore: Upgrade to the latest API spec (from staging, for now)
* chore: RunParameter becomes RunParameterInput
* chore: Build failure prevented python client generation
* chore: Refactor how we track run status and add `starting`.
Fix some broken test infra for the new starting properties, too.
* chore: Fix formatting
* chore: Add `starting` to the rust side, too
* chore: Regenerate API clients against prod schema
Refresh Python and Rust clients from api.tower.dev now that the
starting state has shipped. Adapt tower-cmd call sites: revert
RunParameterInput -> RunParameter (prod kept the original name) and
pass environment: None for the new optional DescribeAppParams field.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(tests): add new starting fields to mock API server
The OpenAPI spec bump to v0.10.24 made `Run.starting_at` and
`RunResults.starting` required (the former is `Option::deserialize`,
so the JSON key must be present even if null). The mock server
wasn't emitting them, so the new generated client deserialization
fell into `UnknownValue` and surfaced as 204/"wasn't able to
understand" errors across the integration suite.
* chore: Add cancelled_child_runs to mock cancel response
Regenerated CancelRunResponse now has a required cancelled_child_runs
field; mock server was returning only {"run": ...} which broke
deserialization and produced empty CLI output in integration tests.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.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:
📝 WalkthroughWalkthroughBumps workspace/project versions and regenerates OpenAPI clients (v0.10.24); removes authenticator/password-reset/verify-email endpoints; adds update-app-environment and organization-usage time-series APIs/models; extends run models (starting timestamps/counters/status); adds client-side pagination aggregation; threads environment/all_environments through deploy/describe; refactors UV/setup retry; updates CLI, mock server, tests, and fixtures. ChangesAPI regeneration, clients, runtime, and CLI (single cohesive cohort)
sequenceDiagram
participant CLI
participant RustAPI as "crates/tower-cmd/api.rs"
participant GenClient as "generated client (rust/python)"
participant Server as "API / mock server"
CLI->>RustAPI: list_apps(--environment?)
RustAPI->>GenClient: fetch_all_pages(page=1..N)
GenClient->>Server: GET /v1/apps?page=1&page_size=...
Server-->>GenClient: { apps: [...], pages: N }
GenClient-->>RustAPI: aggregated Vec<AppSummary>
RustAPI-->>CLI: render table / json
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/apps.rs`:
- Around line 625-630: The test test_terminal_statuses_explicit defines a local
non_terminal array that omits Status::Starting, so add Status::Starting to that
non_terminal list ( alongside Status::Scheduled, Status::Pending,
Status::Running, Status::Retrying ) to ensure is_run_finished is verified to
treat Starting as non-terminal; update the non_terminal declaration in that test
to include Status::Starting.
In `@crates/tower-runtime/src/local.rs`:
- Around line 267-279: The retry logic currently retries on any non-zero exit
(res) including cancellation (-1); update the conditional around the retry (the
block that sends the Output and calls uv.sync_with_legacy_setuptools_pin and
run_setup_child) to skip retry when res indicates cancellation (check for res ==
-1 or the appropriate cancelled sentinel) so that you only retry when res != 0
&& res != -1; keep the existing output/send behavior for real failures but do
not spawn retry_child or call run_setup_child when cancel_token/cancelled exit
is detected.
🪄 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: 34cd20a9-b6ab-40ee-ba74-cace531eb8ba
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (279)
Cargo.tomlcrates/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_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_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_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_organization_usage_time_series_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_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_parameter_input.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/update_account_params.rscrates/tower-api/src/models/update_account_response.rscrates/tower-api/src/models/update_app_environment_params.rscrates/tower-api/src/models/update_app_environment_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_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/usage_metric_time_series_point.rscrates/tower-api/src/models/user.rscrates/tower-api/src/models/webhook.rscrates/tower-cmd/src/api.rscrates/tower-cmd/src/apps.rscrates/tower-cmd/src/deploy.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/run.rscrates/tower-cmd/src/schedules.rscrates/tower-cmd/src/secrets.rscrates/tower-cmd/src/session.rscrates/tower-cmd/src/skill.rscrates/tower-cmd/src/teams.rscrates/tower-cmd/src/util/apps.rscrates/tower-cmd/src/util/deploy.rscrates/tower-package/src/core.rscrates/tower-package/src/native.rscrates/tower-package/src/towerfile.rscrates/tower-package/src/wasm.rscrates/tower-package/tests/package_test.rscrates/tower-runtime/src/lib.rscrates/tower-runtime/src/local.rscrates/tower-telemetry/src/logging.rscrates/tower-uv/src/lib.rscrates/tower-uv/tests/sync_test.rspyproject.tomlsrc/tower/_client.pysrc/tower/tower_api_client/api/default/create_password_reset.pysrc/tower/tower_api_client/api/default/delete_authenticator.pysrc/tower/tower_api_client/api/default/deploy_app.pysrc/tower/tower_api_client/api/default/describe_app.pysrc/tower/tower_api_client/api/default/describe_organization_usage.pysrc/tower/tower_api_client/api/default/generate_authenticator.pysrc/tower/tower_api_client/api/default/generate_organization_usage_time_series.pysrc/tower/tower_api_client/api/default/resend_email_verification.pysrc/tower/tower_api_client/api/default/update_app_environment.pysrc/tower/tower_api_client/api/default/update_password_reset.pysrc/tower/tower_api_client/api/default/verify_email.pysrc/tower/tower_api_client/models/__init__.pysrc/tower/tower_api_client/models/cancel_run_response.pysrc/tower/tower_api_client/models/create_authenticator_params.pysrc/tower/tower_api_client/models/create_authenticator_response.pysrc/tower/tower_api_client/models/create_catalog_params_type.pysrc/tower/tower_api_client/models/create_password_reset_params.pysrc/tower/tower_api_client/models/create_password_reset_response.pysrc/tower/tower_api_client/models/create_schedule_params.pysrc/tower/tower_api_client/models/delete_authenticator_response.pysrc/tower/tower_api_client/models/generate_authenticator_response.pysrc/tower/tower_api_client/models/generate_organization_usage_time_series_response.pysrc/tower/tower_api_client/models/generate_run_statistics_status_item.pysrc/tower/tower_api_client/models/list_authenticators_response.pysrc/tower/tower_api_client/models/list_runs_status_item.pysrc/tower/tower_api_client/models/run.pysrc/tower/tower_api_client/models/run_attempt.pysrc/tower/tower_api_client/models/run_parameter.pysrc/tower/tower_api_client/models/run_results.pysrc/tower/tower_api_client/models/run_status.pysrc/tower/tower_api_client/models/run_timeseries_point.pysrc/tower/tower_api_client/models/schedule.pysrc/tower/tower_api_client/models/search_runs_status_item.pysrc/tower/tower_api_client/models/unverified_authenticator.pysrc/tower/tower_api_client/models/update_app_environment_params.pysrc/tower/tower_api_client/models/update_app_environment_response.pysrc/tower/tower_api_client/models/update_password_reset_params.pysrc/tower/tower_api_client/models/update_schedule_params.pysrc/tower/tower_api_client/models/usage_metric_time_series_point.pysrc/tower/tower_api_client/models/usage_metric_time_series_point_name.pysrc/tower/tower_api_client/models/verified_authenticator.pysrc/tower/tower_api_client/models/verify_email_params.pysrc/tower/tower_api_client/models/verify_email_response.pytests/mock-api-server/main.pytests/tower/test_client.py
💤 Files with no reviewable changes (18)
- src/tower/tower_api_client/models/update_password_reset_params.py
- src/tower/tower_api_client/api/default/generate_authenticator.py
- src/tower/tower_api_client/models/generate_authenticator_response.py
- src/tower/tower_api_client/models/create_password_reset_response.py
- src/tower/tower_api_client/models/verify_email_response.py
- src/tower/tower_api_client/models/unverified_authenticator.py
- src/tower/tower_api_client/api/default/create_password_reset.py
- src/tower/tower_api_client/models/verified_authenticator.py
- src/tower/tower_api_client/models/create_authenticator_response.py
- src/tower/tower_api_client/api/default/update_password_reset.py
- src/tower/tower_api_client/models/verify_email_params.py
- src/tower/tower_api_client/models/delete_authenticator_response.py
- src/tower/tower_api_client/models/create_authenticator_params.py
- src/tower/tower_api_client/api/default/delete_authenticator.py
- src/tower/tower_api_client/models/list_authenticators_response.py
- src/tower/tower_api_client/api/default/verify_email.py
- src/tower/tower_api_client/models/create_password_reset_params.py
- src/tower/tower_api_client/api/default/resend_email_verification.py
Comment on lines
+625
to
+630
| let non_terminal = [ | ||
| Status::Scheduled, | ||
| Status::Pending, | ||
| Status::Running, | ||
| Status::Retrying, | ||
| ]; |
There was a problem hiding this comment.
Add Status::Starting to non-terminal coverage in test_terminal_statuses_explicit.
Line 625’s non_terminal set omits Status::Starting, so a regression where is_run_finished incorrectly treats Starting as terminal would not be caught here.
Proposed fix
let non_terminal = [
Status::Scheduled,
Status::Pending,
+ Status::Starting,
Status::Running,
Status::Retrying,
];📝 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
| let non_terminal = [ | |
| Status::Scheduled, | |
| Status::Pending, | |
| Status::Running, | |
| Status::Retrying, | |
| ]; | |
| let non_terminal = [ | |
| Status::Scheduled, | |
| Status::Pending, | |
| Status::Starting, | |
| Status::Running, | |
| Status::Retrying, | |
| ]; |
🤖 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 625 - 630, The test
test_terminal_statuses_explicit defines a local non_terminal array that omits
Status::Starting, so add Status::Starting to that non_terminal list ( alongside
Status::Scheduled, Status::Pending, Status::Running, Status::Retrying ) to
ensure is_run_finished is verified to treat Starting as non-terminal; update the
non_terminal declaration in that test to include Status::Starting.
* fix: fetch all pages in list commands via reusable pagination helper All list commands (apps, teams, secrets, catalogs, environments, schedules) only fetched the first page from the API, silently truncating results. Added a PaginatedResponse trait and generic fetch_all_pages helper that iterates through all pages. Both CLI and MCP tools benefit since they share the same api.rs layer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add pagination integration tests and mock server support - Mock server now respects page/page_size query params on all list endpoints - Added paginate() helper and mock_max_page_size override for testing - Added /test/seed-apps and /test/reset endpoints for test data setup - Added cli_pagination.feature with scenarios verifying multi-page fetching Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: pagination test accounts for pre-existing test app The mock server re-adds predeployed-test-app on reset, so JSON mode returns 26 (25 seeded + 1 pre-existing). Changed assertion to "at least 25". Also pinned mock server to Python <3.14 (pyo3 compat). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Fix python 3.14 is too new for pyo3 bindings * style: fix black formatting in test files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: avoid race condition in pagination test by using natural page overflow Instead of capping page_size server-side (shared mutable state across parallel workers), seed 105 apps so pagination naturally exceeds the CLI's page_size=100, producing 2 pages without any global state changes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: improve error diagnostics for pagination deserialization failures Include truncated response content in error messages to help debug CI-only deserialization failures. Remove unused debug step from tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: include serde deserialization error in diagnostic message This will reveal the exact field/type mismatch causing CI pagination test failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: run pagination tests serially to avoid shared mock state races Tag cli_pagination.feature with @serial and split the test runner into two phases: parallel features first (excluding @serial), then serial features without --jobs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: strip run_results from mock list response to match real API The real Tower API does not include run_results in list app responses. Including it in the mock caused deserialization failures on CI where a cached wheel expected a newer RunResults schema with a 'starting' field. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: fix black formatting in mock server Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tests/integration/run_tests.py`:
- Around line 199-210: serial_args currently only removes separate "--jobs N" /
"-j N" tokens, allowing inline forms like "--jobs=2" or "-j2" through; update
the serial_args construction in tests/integration/run_tests.py to also filter
out tokens that start with "--jobs=" and tokens that match the "-j<digits>"
pattern (e.g. "-j2" or "-j10"), or equivalently normalize args first and drop
any arg that matches r"^--jobs(=.*)?$" or r"^-j\d+$"; ensure you remove the
initial unused list comprehension and keep the single loop or a single
comprehension that excludes ("--jobs","-j"), their inline forms, and
numeric-only tokens so that `@serial` runs never see any jobs flags.
In `@tests/mock-api-server/main.py`:
- Around line 801-809: The run_results dict created by the /test/reset handler
omits the starting field, causing inconsistency with the initial app definition;
locate the run_results dictionary in the reset endpoint (the block that
re-creates the predeployed-test-app) and add the "starting": 0 key/value
alongside cancelled, crashed, errored, exited, pending, retrying, and running so
the reset payload matches the original definition.
- Around line 760-768: Seeded app dictionaries are missing the new "starting"
key inside their "run_results" dict; update the seeded app entries that define
"run_results" (the same dict structure used for the pre-populated app and newly
created apps) to include "starting": 0 so the shape matches the pre-populated
app and new app creation. Locate the seeded apps in
tests/mock-api-server/main.py where "run_results" is constructed for seed data
and add the "starting" field with a zero value to each run_results object to
ensure consistency with the app created in the other code paths.
🪄 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: 65a8ef6c-144d-46cc-9024-0e4f50490583
⛔ Files ignored due to path filters (1)
tests/mock-api-server/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
crates/tower-cmd/src/api.rscrates/tower-cmd/src/apps.rscrates/tower-cmd/src/catalogs.rscrates/tower-cmd/src/environments.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/schedules.rscrates/tower-cmd/src/secrets.rscrates/tower-cmd/src/teams.rstests/integration/features/cli_pagination.featuretests/integration/features/steps/cli_steps.pytests/integration/run_tests.pytests/mock-api-server/main.pytests/mock-api-server/pyproject.tomltests/mock-api-server/tower_mock_api.egg-info/PKG-INFO
✅ Files skipped from review due to trivial changes (4)
- tests/mock-api-server/tower_mock_api.egg-info/PKG-INFO
- tests/mock-api-server/pyproject.toml
- crates/tower-cmd/src/catalogs.rs
- crates/tower-cmd/src/teams.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/tower-cmd/src/apps.rs
Comment on lines
+199
to
+210
| serial_args = [a for a in args if a not in ("--jobs", "-j") and not a.isdigit()] | ||
| # Remove --jobs N pair from args | ||
| serial_args = [] | ||
| skip_next = False | ||
| for a in args: | ||
| if skip_next: | ||
| skip_next = False | ||
| continue | ||
| if a in ("--jobs", "-j"): | ||
| skip_next = True | ||
| continue | ||
| serial_args.append(a) |
There was a problem hiding this comment.
Handle inline jobs flags when building serial_args.
serial_args removes --jobs N / -j N, but inline forms (for example --jobs=2 or -j2) still pass through. That can keep parallelization enabled during the @serial phase.
Suggested patch
- serial_args = [a for a in args if a not in ("--jobs", "-j") and not a.isdigit()]
- # Remove --jobs N pair from args
+ # Remove all parallelism flags from args for `@serial` run.
serial_args = []
skip_next = False
for a in args:
if skip_next:
skip_next = False
continue
if a in ("--jobs", "-j"):
skip_next = True
continue
+ if a.startswith("--jobs="):
+ continue
+ if a.startswith("-j") and len(a) > 2 and a[2:].isdigit():
+ continue
serial_args.append(a)🤖 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 `@tests/integration/run_tests.py` around lines 199 - 210, serial_args currently
only removes separate "--jobs N" / "-j N" tokens, allowing inline forms like
"--jobs=2" or "-j2" through; update the serial_args construction in
tests/integration/run_tests.py to also filter out tokens that start with
"--jobs=" and tokens that match the "-j<digits>" pattern (e.g. "-j2" or "-j10"),
or equivalently normalize args first and drop any arg that matches
r"^--jobs(=.*)?$" or r"^-j\d+$"; ensure you remove the initial unused list
comprehension and keep the single loop or a single comprehension that excludes
("--jobs","-j"), their inline forms, and numeric-only tokens so that `@serial`
runs never see any jobs flags.
* feat: add environment support to apps list/show CLI and MCP tools The Tower API scopes apps to environments but the CLI and MCP tools were not fully exposing this. This commit: - Adds --environment/-e flag to `tower apps list` and `tower apps show` (defaulting to "default") - Adds environment parameter to MCP tools: tower_deploy, tower_apps_list, tower_apps_show, and tower_run_remote - Adds new MCP tools: tower_catalogs_list and tower_catalogs_show - Includes CLI arg parsing tests for the new flags Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: default apps list to no environment filter The API returns all apps when environment is omitted, and only filters when explicitly provided. Updated list_apps to take Option<&str> so the default behavior shows all apps across environments. The environment param is still available for explicit filtering. Also added version field to MCP apps list output. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: codingcyclist <41942954+codingcyclist@users.noreply.github.com>
…28 (#277) * ci: bump aarch64 wheel base from manylinux2014 to manylinux_2_28 The aarch64 linux-cross job built wheels in the CentOS 7 / GCC 4.8 toolchain that ships with manylinux2014. Switching to manylinux_2_28 (AlmaLinux 8, GCC 8+) gives the build a newer assembler, which avoids a class of miscompilations in crates that ship hand-written aarch64 asm. * fix: switch rustls CryptoProvider from ring to aws-lc-rs reqwest's rustls-tls feature implicitly enables rustls/ring, which pulls ring v0.17 into the TLS path. ring's aarch64 build produces TLS signature-verify failures on some glibc builds of our wheels. Swap every reqwest TLS feature to the *-no-provider variant and install aws-lc-rs as the process-wide default CryptoProvider at CLI startup. The install is also called at the tower-uv reqwest entry point so its standalone unit test (which bypasses App::new) has a provider installed before the first TLS handshake. After this change, `cargo tree -i ring` is empty. * fix: lock in tower-api template + drop dead aarch64 CFLAGS scripts/rust-client-templates/Cargo.mustache hardcodes `rustls-tls` in the generated reqwest dep. Patch the template so the next generate-rust-api-client.sh run preserves the aws-lc-rs feature choice instead of reverting tower-api's Cargo.toml. The CFLAGS_aarch64_unknown_linux_gnu workaround in build-binaries.yml existed only to coax ring's build script into detecting ARMv8. With ring gone from the dep tree, the workaround is dead.
…umbing (#274) `LocalApp::start` previously spawned `execute_local_app` with a `oneshot::Sender<i32>`; every `?` early-return in that task dropped the sender silently, and `LocalApp::status()` reported the resulting closed channel as `Error::WaiterClosed` — losing the actual cause (env join errors, `tower_uv::Error` variants, panics, etc.). Replace the channel payload with an internal `AppCompletion` enum and a new `Status::Failed(AppFailure)` shape that preserves the structured error. Highlights: - `AppCompletion` (internal) has `Exit(i32)`, `Cancelled`, `Failed(AppFailure)` variants. The spawn wrapper in `LocalApp::start` runs `inner_execute_local_app` inside `AssertUnwindSafe(...).catch_unwind()` and always sends an `AppCompletion` before exiting, so a closed waiter channel can no longer mask a real failure. - `Status::Failed(AppFailure)` replaces the old `Status::Failed { error_code, error_message }` strings. `AppFailure` has: * `Runtime(Error)` — structured runtime error * `Panic(String)` — `catch_unwind` payload * `Platform { error_code, error_message }` — opaque strings for callers that construct `Status::Failed` from outside `tower_runtime` (e.g. Kubernetes pod-status reconciliation). Stringification (`error_code` / `error_message`) is the consumer's responsibility, not the runtime's. - `Status::Cancelled` is a new terminal variant for explicit cancellation via the `CancellationToken`. Previously cancellation was represented as `Crashed { code: -1 }`, indistinguishable from an app that legitimately exits with code 255 / -1 or an IO error in `wait_for_process`. `inner_execute_local_app` now returns `Err(Error::Cancelled)` on every cancellation path (4 prologue checks + 3 post-`wait_for_process` checks that disambiguate cancellation from a child's genuine non-zero exit), and the spawn wrapper special-cases that into `AppCompletion::Cancelled`. - `Error` gains `Clone + PartialEq + Eq` (all variants are unit, so this is free) so `Status` can keep deriving `Clone + PartialEq`, which is required by status caching in `LocalApp::status()` and by existing integration tests. - `tower-cmd` (the only other in-crate consumer of `Status::Failed`) is updated for the new variant shape and prints a dedicated "cancelled" message for `Status::Cancelled`. - The integration test `test_abort_on_dependency_installation_failure` keeps asserting that `uv sync` non-zero exits surface as `Status::Crashed { code }` — `Status::Failed` is reserved for platform failures where the child never ran. `Error::WaiterClosed` now only fires when the spawn was aborted or the runtime was dropped — both real bugs worth surfacing as-is.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/tower-runtime/tests/local_test.rs (1)
363-380:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix the stale
Status::Failedpattern in this match.Line 379 uses
Status::Failed { .. }, butStatusdefinesFailed(AppFailure)as a tuple variant, causing a compile error.Proposed fix
- Status::Failed { .. } => { + Status::Failed(_) => { panic!("App should have crashed, not failed with a platform error"); }🤖 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-runtime/tests/local_test.rs` around lines 363 - 380, The match arm uses the outdated struct-style pattern Status::Failed { .. } which doesn't match the enum's tuple variant; update the match to use the tuple pattern (e.g., Status::Failed(_) or Status::Failed(_err)) in the match inside the test (the match over Status in tests/local_test.rs) so it compiles against the enum definition (Status::Failed(AppFailure)).
🤖 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/run.rs`:
- Around line 254-270: Update monitor_cli_status() to match the new Status enum
shape and treat cancellations as terminal: replace the old pattern
Status::Failed { .. } with the tuple/variant pattern Status::Failed(_) in the
match arm that handles failures (so it compiles against the new
tower_runtime::Status), and add a branch that treats Status::Cancelled as a
terminal state (exiting the polling loop and returning/propagating the
cancellation path so the existing cancellation handling in the other function
runs). Ensure you reference the same match in monitor_cli_status() and update
any control flow there to break/return when encountering Status::Cancelled.
In `@crates/tower-runtime/src/local.rs`:
- Around line 135-146: The code is treating any error from
wait_for_process()/child.wait() as the sentinel Ok(-1) via run_setup_child(),
which hides real I/O runtime failures and turns them into Status::Crashed {
code: -1 } instead of Status::Failed(AppFailure::Runtime(_)); change
run_setup_child() and inner_execute_local_app() so that CancellationToken
cancellation still maps to Ok(-1) but genuine child.wait()/wait_for_process()
I/O errors are propagated as Err(Error::Runtime(...)) (or the crate's runtime
failure variant) rather than converted to an exit-code integer—specifically,
detect token.is_cancelled() to return the -1 path but return Err for any Err
returned from wait_for_process()/child.wait(), and update callers that currently
expect Ok(i32) from run_setup_child() to handle Err(Error) accordingly so the
failure surfaces as AppFailure::Runtime.
---
Outside diff comments:
In `@crates/tower-runtime/tests/local_test.rs`:
- Around line 363-380: The match arm uses the outdated struct-style pattern
Status::Failed { .. } which doesn't match the enum's tuple variant; update the
match to use the tuple pattern (e.g., Status::Failed(_) or Status::Failed(_err))
in the match inside the test (the match over Status in tests/local_test.rs) so
it compiles against the enum definition (Status::Failed(AppFailure)).
🪄 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: f99ac759-8a6d-4c98-a976-548e5457dd01
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
crates/tower-cmd/src/run.rscrates/tower-runtime/Cargo.tomlcrates/tower-runtime/src/errors.rscrates/tower-runtime/src/lib.rscrates/tower-runtime/src/local.rscrates/tower-runtime/tests/local_test.rs
✅ Files skipped from review due to trivial changes (2)
- crates/tower-runtime/Cargo.toml
- crates/tower-runtime/src/errors.rs
* fix: align pagination with API semantics and make it configurable The CLI pagination loop added in #275 was 0-indexed but the API treats page=0 (or null) as 'return everything in one response' and uses 1-indexed pages otherwise. The loop happened to work against real prod only because the first request asked for 'all' and immediately exited the loop. The mock server was 0-indexed and so diverged from the real API's semantics. Changes: - Add four hidden global flags for paginated list commands: --page-size, --page, --no-paginate, --max-items. Modeled on the AWS CLI's pagination knobs but matched to our page-number API (--starting-token would be cursor-shaped, which our API does not use). - Thread page_size / paginate / page / max_items through Config. - Fix fetch_all_pages: page=0 means 'all in one response and stop'; page>=1 iterates 1..=num_pages. - Align the mock /v1/apps/etc. pagination to the same semantics so the mock and prod behave the same way. - Rewrite cli_pagination.feature to create apps via the real CLI and delete them in after_scenario. With --page 1 --page-size 2 against three apps, the test forces a real two-page traversal against any backend. - Drop the mock-only /test/seed-apps and /test/reset endpoints that the old pagination feature relied on. * chore: Reformat the things * chore: Clean up old, broken rust that comes with Homebrew * chore: Go the other way with it--clobber the Homebrew rustup * chore: One more attempt to fix this broken rust toolchain in macos15 images * chore: Another attempt at resolving the issue with the upstream --------- Co-authored-by: Brad Heller <brad@tower.dev>
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
Release
New Features
Improvements
Removed