feat(task): scheduled pipeline deployments (deploy v2)#1073
Open
stepmikhaylov wants to merge 5 commits into
Open
feat(task): scheduled pipeline deployments (deploy v2)#1073stepmikhaylov wants to merge 5 commits into
stepmikhaylov wants to merge 5 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughRefactors deployment records and storage, separates user credentials from client-visible payloads, introduces an in-process task execution facade, revises TaskScheduler API and lifecycle, wires scheduler into deploy commands and module startup/shutdown, and aligns Python/TypeScript clients and docs to camelCase deployment fields. ChangesDeployment scheduling refactor with credential separation and client SDK alignment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
module:client-python
|
No description provided. |
stepmikhaylov
force-pushed
the
feat/deploy-v2
branch
2 times, most recently
from
3e25a2e to
44ebff6
Compare
github-actions
Bot
removed
module:client-python
stepmikhaylov
force-pushed
the
feat/deploy-v2
branch
4 times, most recently
from
d08da2d to
bf38cc3
Compare
github-actions
Bot
added
module:client-python
stepmikhaylov
force-pushed
the
feat/deploy-v2
branch
3 times, most recently
from
3eefd52 to
96fe90d
Compare
stepmikhaylov
force-pushed
the
feat/deploy-v2
branch
from
96fe90d to
2c78da1
Compare
stepmikhaylov
force-pushed
the
feat/deploy-v2
branch
from
2c78da1 to
03c26ba
Compare
stepmikhaylov
changed the title
stepmikhaylov
requested review from
Rod-Christensen and
jmaionchi
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
packages/ai/src/ai/modules/task/commands/cmd_deploy.py (1)
177-208: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider returning the updated record for consistency with
add.
rrext_deploy_addreturnsrecord.to_client_record()whilerrext_deploy_updatereturns an empty body. This inconsistency requires clients to make a follow-upstatus()call afterupdate()to get the newupdatedAttimestamp. If this is intentional (e.g., to avoid redundant serialization), consider documenting the API contract difference.🤖 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 `@packages/ai/src/ai/modules/task/commands/cmd_deploy.py` around lines 177 - 208, The update handler on_rrext_deploy_update currently returns an empty body while rrext_deploy_add returns record.to_client_record(); change on_rrext_deploy_update to return the updated deployment in the response (e.g., build_response(request, body=record.to_client_record())) after save() and scheduling so clients receive the new updatedAt and other client fields consistently with rrext_deploy_add; ensure the record object provides to_client_record() or serialize it the same way rrext_deploy_add does.
🤖 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 `@packages/ai/src/ai/modules/task/task_server_facade.py`:
- Around line 59-64: The current `exec_response` handling in the code that
awaits `conn.request('execute', arguments={'pipeline': pipeline})` only checks
top-level success and then indexes `exec_response['body']['token']`, which can
raise KeyError if `body` or `token` is missing; add defensive guards after the
success check to verify `exec_response.get('body')` is a dict and that
`exec_response['body'].get('token')` exists, and if not raise a clear
RuntimeError (e.g., "execute missing body" or "execute missing token") so
callers get a descriptive error instead of a KeyError.
In `@packages/client-python/src/rocketride/types/deploy.py`:
- Around line 35-49: Add or extend the Python SDK docs to document the deploy
API and the DeploymentRecord schema used by client.deploy.add,
client.deploy.list, and client.deploy.status: create a new documentation page
under packages/client-python/docs (e.g., deploy/deployment-record.md) or extend
index.md to include the DeploymentRecord fields and types, explicitly listing
pipeline: PipelineConfig, schedule: str, state:
Literal['active','paused','errored'], userId: str, createdAt: float, and
updatedAt: float, and ensure the public SDK signatures shown in the docs match
these exact field names.
---
Outside diff comments:
In `@packages/ai/src/ai/modules/task/commands/cmd_deploy.py`:
- Around line 177-208: The update handler on_rrext_deploy_update currently
returns an empty body while rrext_deploy_add returns record.to_client_record();
change on_rrext_deploy_update to return the updated deployment in the response
(e.g., build_response(request, body=record.to_client_record())) after save() and
scheduling so clients receive the new updatedAt and other client fields
consistently with rrext_deploy_add; ensure the record object provides
to_client_record() or serialize it the same way rrext_deploy_add does.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f1b32a27-61ab-4808-858f-366db128eb3a
📒 Files selected for processing (16)
packages/ai/src/ai/account/deployment_store.pypackages/ai/src/ai/account/models.pypackages/ai/src/ai/modules/task/__init__.pypackages/ai/src/ai/modules/task/commands/cmd_deploy.pypackages/ai/src/ai/modules/task/task_scheduler.pypackages/ai/src/ai/modules/task/task_server.pypackages/ai/src/ai/modules/task/task_server_facade.pypackages/ai/tests/ai/account/test_deployment_store.pypackages/ai/tests/ai/modules/task/test_task_scheduler.pypackages/client-python/src/rocketride/deploy.pypackages/client-python/src/rocketride/types/deploy.pypackages/client-python/tests/test_deploy.pypackages/client-typescript/docs/guide/methods/deploy.mdpackages/client-typescript/src/client/deploy.tspackages/client-typescript/src/client/types/deploy.tspackages/client-typescript/tests/deploy.test.ts
💤 Files with no reviewable changes (1)
- packages/ai/src/ai/modules/task/task_server.py
stepmikhaylov
force-pushed
the
feat/deploy-v2
branch
from
03c26ba to
10065ba
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client-python/docs/index.md (1)
24-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign the Python methods table with the real SDK signature.
The table still uses JS-style
options?andprojectId, but the Python client exposes keyword-only args (schedule,pipeline) andproject_id. That mismatch will mislead Python users and contradict the API shown below.Proposed fix
-| `deploy.add(pipeline, options?)` | Persist a pipeline as a deployment and activate it | -| `deploy.remove(projectId)` | Undeploy and delete the deployment | +| `deploy.add(pipeline, *, schedule=None)` | Persist a pipeline as a deployment and activate it | +| `deploy.remove(project_id)` | Undeploy and delete the deployment | | `deploy.list()` | List the authenticated user's deployments | -| `deploy.status(projectId)` | Get one deployment record | -| `deploy.update(projectId, options?)` | Replace the pipeline and/or schedule | +| `deploy.status(project_id)` | Get one deployment record | +| `deploy.update(project_id, *, pipeline=None, schedule=None)` | Replace the pipeline and/or schedule |🤖 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 `@packages/client-python/docs/index.md` around lines 24 - 30, The methods table in the Python docs is using JS-style signatures (e.g., options?, projectId) which don't match the Python SDK; update the table to reflect the Python client signatures (use keyword-only parameters like schedule and pipeline and snake_case project_id) and ensure entries referencing RocketRideClient methods match the actual Python function/method names and parameter styles (e.g., schedule, pipeline, project_id) so examples and parameter docs align with the SDK.
🤖 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.
Outside diff comments:
In `@packages/client-python/docs/index.md`:
- Around line 24-30: The methods table in the Python docs is using JS-style
signatures (e.g., options?, projectId) which don't match the Python SDK; update
the table to reflect the Python client signatures (use keyword-only parameters
like schedule and pipeline and snake_case project_id) and ensure entries
referencing RocketRideClient methods match the actual Python function/method
names and parameter styles (e.g., schedule, pipeline, project_id) so examples
and parameter docs align with the SDK.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e446cba2-90fe-485b-b0a5-bdaece78aac2
📒 Files selected for processing (2)
packages/client-python/docs/index.mdpackages/client-typescript/docs/guide/methods/deploy.md
stepmikhaylov
force-pushed
the
feat/deploy-v2
branch
from
10065ba to
c04a449
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client-python/tests/test_deploy.py (1)
101-112: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAssert credential redaction on
list()andstatus()too.The leak-prevention check only covers
add(), but the contract here is broader: stored deployment credentials must never come back from any deploy read path. Please add the sameuserToken-absence assertions forstatus()and for each record returned bylist()in both SDK integration suites.Based on the PR objective, the redaction guarantee applies to the full deploy lifecycle API, not just creation.
🤖 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 `@packages/client-python/tests/test_deploy.py` around lines 101 - 112, The test currently asserts that add() does not echo stored credentials but misses the same guarantees for other read paths; update the tests (starting from test_add_returns_full_record) to call deploy.list() and iterate each returned record asserting 'userToken' not in rec, and also call deploy.status(rec_id) for a created record and assert 'userToken' not in that status response; apply the same assertions in the other SDK integration test suites that exercise deploy.list() and deploy.status() so all read paths (add, list, status) enforce credential redaction.
🤖 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 `@packages/client-python/docs/index.md`:
- Line 258: The docs text about deployment States incorrectly implies that state
== 'active' always means cron ticks are firing; update the wording in
packages/client-python/docs/index.md to clarify that 'active' can also apply to
"manual" deployments (i.e., deployments with no schedule or schedule == 'manual'
that do not perform scheduled cron ticks) — reference the 'state' field and the
specific values 'manual', 'active', 'paused', and 'errored' and indicate that
manual deployments report state == 'active' but do not run scheduled ticks so
the default schedule may look like 'paused' even when state is 'active'.
---
Outside diff comments:
In `@packages/client-python/tests/test_deploy.py`:
- Around line 101-112: The test currently asserts that add() does not echo
stored credentials but misses the same guarantees for other read paths; update
the tests (starting from test_add_returns_full_record) to call deploy.list() and
iterate each returned record asserting 'userToken' not in rec, and also call
deploy.status(rec_id) for a created record and assert 'userToken' not in that
status response; apply the same assertions in the other SDK integration test
suites that exercise deploy.list() and deploy.status() so all read paths (add,
list, status) enforce credential redaction.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: cb3f0205-cf1b-44a1-8442-82554f1f8c44
📒 Files selected for processing (14)
packages/ai/src/ai/modules/task/__init__.pypackages/ai/src/ai/modules/task/commands/cmd_deploy.pypackages/ai/src/ai/modules/task/task_scheduler.pypackages/ai/src/ai/modules/task/task_server.pypackages/ai/src/ai/modules/task/task_server_facade.pypackages/ai/tests/ai/modules/task/test_task_scheduler.pypackages/client-python/docs/index.mdpackages/client-python/src/rocketride/deploy.pypackages/client-python/src/rocketride/types/deploy.pypackages/client-python/tests/test_deploy.pypackages/client-typescript/docs/guide/methods/deploy.mdpackages/client-typescript/src/client/deploy.tspackages/client-typescript/src/client/types/deploy.tspackages/client-typescript/tests/deploy.test.ts
💤 Files with no reviewable changes (1)
- packages/ai/src/ai/modules/task/task_server.py
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
1 participant
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.
Deploy v2 — scheduled pipeline deployments
Pipelines can now be deployed to the server and run autonomously on a schedule. A deployment persists a pipeline with a cron schedule (
*/15 * * * *,@hourly, … ormanual); the built-in scheduler fires due runs under the deploying user's account, with the same authentication, permission, and plan checks as an on-demand run.Highlights
rrext_deploy_add / update / list / status / remove.erroredand stops scheduling instead of retrying forever.Clients ready to build on
The Python and TypeScript SDKs ship a complete, typed
client.deploynamespace (add/remove/list/status/update) with integration tests and a docs page — fully capable for on-top development (SaaS UI and beyond).Not the final version — remaining work
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores