Repository navigation
fix(cli): hydrate acquired deployment configuration - #380
Merged
Merged
Conversation
Greptile SummaryThe PR preserves the authoritative deployment configuration returned during acquisition and makes target-release resolution fail closed.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| crates/alien-cli/src/commands/deploy.rs | Hydrates acquired deployment configuration, restores runtime connection fields, and validates target-release retrieval and platform stack selection. |
Sequence Diagram
sequenceDiagram
participant CLI as alien CLI
participant Manager as Deployment Manager
participant Runner as Deployment Runner
CLI->>Manager: Fetch deployment
CLI->>Manager: Fetch desired release
Manager-->>CLI: Platform-specific release stack
CLI->>Manager: Acquire deployment lock
Manager-->>CLI: Acquired deploymentConfig
CLI->>CLI: Hydrate config and restore runtime fields
CLI->>Runner: Run deployment step loop
Runner->>Manager: Reconcile state
CLI->>Manager: Release deployment lock
Reviews (3): Last reviewed commit: "fix(cli): hydrate acquired deployment co..." | Re-trigger Greptile
alongubkin
force-pushed
the
alon/alien-478-fix-cli-setup-deploy-hydration
branch
from
August 13, 2026 02:35
da5026b to
739cef0
Compare
alongubkin
added a commit
that referenced
this pull request
Oct 14, 2026
) ## Problem When the manager starts an update and `alien deploy` takes the lease and finishes it, every container is rolled twice. The second rollout uses a different spec: the containers lose `ALIEN_DEPLOYMENT_ID` and `ALIEN_DEPLOYMENT_NAME`, and command-enabled containers and daemons also lose `ALIEN_COMMANDS_*`. They run without these variables until the manager runs another update. For a single-replica database, that means two restarts back to back instead of one. ## Root cause Two runners built two different configs for the same operation: - **Manager loop** (`loops/deployment.rs`): builds an environment snapshot with the deployment id and name, the command receiver variables, and the deployment's own variables. It puts that snapshot in the config before it runs a step. - **`POST /v1/sync/acquire`** (`routes/sync.rs`): returned the record's raw `deploymentConfig`, so the control plane's copy of the variables went out without the manager's snapshot. In standalone mode it returned no config at all. - **`alien deploy`** and **`alien-deploy-cli up`**: both use the acquired config as-is (since #380). The executor sees a different desired config after the first rollout is ready, so it runs a second update. The variables were not left out on purpose. Acquire already hands out the secret-bearing config: the record field is `skip_serializing` precisely so that acquire is the one route that returns it, and the acquire helper's doc says it exists for config fields redacted from `GET /v1/deployments`. Every value in the snapshot comes from data the caller already has, or is entitled to: - the deployment id and name; - the commands token, which is the deployment token the CLI is already authenticated with; - the deployment's variables, which are already on the acquired record. A runner has to write all of these into the workloads anyway. ## Fix One shared builder, `RunnerConfigSources::deployment_config`, used by both the loop and the acquire route. It holds the code the loop already ran, moved out unchanged: the environment snapshot, the management config, monitoring, the native image host, and how a control plane's config gets merged. Target-stack resolution (supplied stack or release stack) moves into `target_release_stack`, which both paths also share. - Acquire now returns the config the loop would run with, in both platform and standalone mode. Neither CLI needed a change. - The BYO Horizon env-var bridge stays loop-only. Its management token belongs to the manager process, and handing it out to every acquirer would be a new exposure. - If the config can't be built (for example, a command-enabled resource with no deployment token), acquire releases the leases it just took and returns the error. Otherwise the caller would hold a lease it doesn't know about, and the manager would be blocked until it expired. - Small cleanup: a missing `stack_settings` is now an error rather than a panic. The tautological `with_environment_snapshot` test is replaced with one that runs the real builder. ## Proof - `tests/acquire_runner_config.rs`: runs the CLI's real acquire helper (`acquire_deployment_with_payload`) against the real `/v1/sync/acquire` route over TCP, using sqlite stores. The deployment has an update queued to a release with a command-enabled container. The test decodes `deploymentConfig` the way `alien deploy` does and asserts the exact variable list: id, name, the four command variables scoped to the container, and the user variable. - **Fails on main** (no `deploymentConfig` in the response) and passes with the fix. - A second test checks that a config that can't be built fails the acquire with `COMMAND_TOKEN_MISSING` and leaves the deployment unlocked. - `routes::sync::tests::acquired_config_is_the_config_the_manager_loop_runs_with`: the reported case, where a control plane supplies the config and stacks. The acquired `deploymentConfig` equals the loop's config field for field; only the snapshot's `createdAt` may differ. - `loops::deployment::tests::runner_config_replaces_control_plane_snapshot_with_manager_variables`: the builder replaces the control plane's snapshot with the manager's variables. Run in a remote sandbox (nightly-2026-10-03): - `cargo nextest run -p alien-manager --lib --test acquire_runner_config`: 334 passed, including the 4 new tests. - Same integration test with `crates/alien-manager/src` from `main`: both tests fail (`acquire returns the config to run with` panics because `deploymentConfig` is missing; the lease test fails because the acquire succeeds). - `cargo clippy -p alien-manager --tests`: no warnings in the changed files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Summary
deploymentConfigreturned by acquisition before entering the deploy loopWhy
The CLI previously rebuilt configuration from local arguments after acquiring a deployment and silently ignored target-release lookup failures. A setup-owned BYO deployment could therefore enter reconciliation without its desired resource graph and complete an observe-only pass instead of applying the release.
The branch is rebased onto current
main, including the remote-debug EOF fix. Stale generated artifacts from the previous branch revision were dropped; current generation produces no schema changes for this Rust-only fix.Verification
rustfmt --edition 2021 --check crates/alien-cli/src/commands/deploy.rscargo test -p alien-cli commands::deploy::tests::target_release_requires_a_stack_for_the_deployment_platformcargo test -p alien-cli commands::deploy::tests::deployment_models_match_platform_deliverypnpm generate(schema generation clean; local protoc 7.34.1 differs from the repository's 7.35.1 only in generated version headers, which were not committed)git diff --checkFresh full CI and automated review are required before merge.