Conversation
|
This PR is targeting If this is a regular feature/fix PR, please change the base branch to Current base: |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes add a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/run_parameter.rs`:
- Around line 23-25: The Python RunParameter model is missing the new boolean
field secret (default False) causing serialization/deserialization drift; update
the RunParameter class to declare a secret: bool = False attribute in the
constructor (and type hints), ensure to_dict() includes the secret key when
serializing, and ensure from_dict()/from_json factory reads secret (using False
if absent) when building the object; regenerate the model from the OpenAPI
schema or manually add these changes in the RunParameter class, its to_dict, and
from_dict/from_json methods to match the Rust model.
In `@crates/tower-cmd/src/api.rs`:
- Around line 843-847: The mapping currently constructs RunParameter with
secret: false, which drops secret metadata; update the map closures that produce
RunParameter (the ones building RunParameter { name: key, value, secret: false
}) to read the secret flag from the incoming request parameter (e.g., use the
source tuple/struct's secret field such as param.1.secret or param.secret) and
set RunParameter.secret accordingly; make the same change at both mapping sites
that create RunParameter so secret parameters are preserved when
creating/updating schedules.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
crates/config/src/towerfile.rscrates/tower-api/src/models/parameter.rscrates/tower-api/src/models/run_parameter.rscrates/tower-cmd/src/api.rscrates/tower-cmd/src/mcp.rscrates/tower/src/lib.rs
| #[serde(default)] | ||
| #[serde(rename = "secret")] | ||
| pub secret: bool, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check if the Rust file exists and show the RunParameter struct
echo "=== Checking Rust RunParameter model ==="
if [ -f crates/tower-api/src/models/run_parameter.rs ]; then
echo "File found. Full content:"
cat -n crates/tower-api/src/models/run_parameter.rs
else
echo "Rust file not found at crates/tower-api/src/models/run_parameter.rs"
fi
echo
echo "=== Checking Python RunParameter model ==="
# Search for Python model in common locations
if [ -f src/tower/tower_api_client/models/run_parameter.py ]; then
echo "File found. Full content:"
cat -n src/tower/tower_api_client/models/run_parameter.py
elif [ -f tower_api_client/models/run_parameter.py ]; then
echo "File found at alternate location. Full content:"
cat -n tower_api_client/models/run_parameter.py
else
echo "Searching for Python RunParameter model:"
find . -name "run_parameter.py" -type f 2>/dev/null || echo "No Python model found"
fiRepository: tower/tower-cli
Length of output: 2569
Regenerate the Python RunParameter model to include the secret field.
The Rust model now includes secret (bool, default false) at lines 23-25, but the Python model at src/tower/tower_api_client/models/run_parameter.py completely omits this field from the class definition, to_dict(), and from_dict() methods. This creates schema drift where the secret metadata is silently lost during serialization and deserialization across client boundaries. Both files are auto-generated by OpenAPI Generator; regenerating the Python model from the same schema will resolve the mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/tower-api/src/models/run_parameter.rs` around lines 23 - 25, The
Python RunParameter model is missing the new boolean field secret (default
False) causing serialization/deserialization drift; update the RunParameter
class to declare a secret: bool = False attribute in the constructor (and type
hints), ensure to_dict() includes the secret key when serializing, and ensure
from_dict()/from_json factory reads secret (using False if absent) when building
the object; regenerate the model from the OpenAPI schema or manually add these
changes in the RunParameter class, its to_dict, and from_dict/from_json methods
to match the Rust model.
| .map(|(key, value)| RunParameter { | ||
| name: key, | ||
| value, | ||
| secret: false, | ||
| }) |
There was a problem hiding this comment.
secret is hard-coded to false for all schedule parameters.
At Line 843 and Line 881 mappings, this prevents creating/updating secret schedule parameters through this path and can downgrade secrecy semantics when parameters are re-sent. The request shape should carry secret metadata instead of forcing false.
Also applies to: 881-885
🤖 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 843 - 847, The mapping currently
constructs RunParameter with secret: false, which drops secret metadata; update
the map closures that produce RunParameter (the ones building RunParameter {
name: key, value, secret: false }) to read the secret flag from the incoming
request parameter (e.g., use the source tuple/struct's secret field such as
param.1.secret or param.secret) and set RunParameter.secret accordingly; make
the same change at both mapping sites that create RunParameter so secret
parameters are preserved when creating/updating schedules.
bradhe
force-pushed
the
feature/secret-parameters
branch
from
1115ab3 to
4dc348b
Compare
1 participant
Summary
Testing