LPX-606: yolo deploy always builds; --app-version names the build#30
Merged
stevethomas merged 2 commits intoMay 20, 2026
Conversation
d83cb1b to
fa2f724
Compare
Restores build-in-deploy that v1's DeployCommand rewrite in #26 lost. Drops the alpha-era assumption that `--app-version=<tag>` meant "deploy this previously-built artefact" — that's rollback's job, not deploy's (future LPX-608). `--app-version` is now an input to the build: the tag to stamp on the fresh image. Absent → BuildCommand generates a timestamp. Present (e.g. a GitHub release name) → BuildCommand uses it verbatim. Build always runs. BuildCommand writes --app-version back onto the shared InputInterface, so deploy's downstream steps pick up the freshly-baked tag via `$this->option('app-version')`. Also drops the `Manifest::has('tasks.web')` guard. The interface mechanism (ExecutesWebStep + Manifest::isHeadless) already gates DNS steps for headless apps. The guard was catching a different scenario — "yolo.yml has no Fargate spec at all" — which is best left to surface as ServiceNotFoundException via the natural AWS error path. README's `yolo init && yolo build && yolo sync production && yolo deploy production` was nonsensical (`yolo build` errored on the missing env arg; explicit build was redundant). Updated to: yolo init && yolo sync production && yolo deploy production Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fa2f724 to
216e4a6
Compare
LPX-604 (#29) added headless-mode gating for solo DNS via the ExecutesWebStep interface, but missed the multitenancy mirror. SyncMultitenancyRecordSetStep dereferences \$this->config['apex'] and ['domain'] unconditionally, so a headless multitenant deploy would undefined-key-crash before reaching ECS. Symmetric fix: implement ExecutesWebStep on the tenant step too, so Manifest::isHeadless() drives the skip via the existing ChecksIfCommandsShouldBeRunning::shouldBeRunning() trait. Co-Authored-By: Claude Opus 4.7 (1M context) <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
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.
Hey, I made a thing! 🥳
LPX-606
What problems are you solving?
yolo deploy <env>now always builds, pushes, and deploys as one motion. The v1 rewrite in PR #26 had inadvertently dropped build-in-deploy, leaving the README's one-liner broken and forcing users to remember a separateyolo buildstep.--app-versionis now reframed as an input to the build — the tag to stamp on the fresh image:yolo deploy production— auto-generates a timestamp tag (26.21.2.1500), builds, pushes, deploys.yolo deploy production --app-version=v1.2.3— usesv1.2.3(e.g. a GitHub release name) as the build's tag, builds, pushes, deploys.The alpha-era "skip build when
--app-versionis supplied" branch is gone. Deploying a pre-existing ECR tag is rollback's job, tracked separately as LPX-608. Folding rollback intodeploy --app-version=<old-tag>muddied the mental model.BuildCommand writes
--app-versionback onto the sharedInputInterfacebefore returning, so deploy's downstream steps pick up the tag via$this->option('app-version').Also drops the
Manifest::has('tasks.web')guard. TheExecutesWebStepinterface +Manifest::isHeadless()already gates DNS steps for headless apps (the right axis). The dropped guard was catching a different scenario — "yolo.yml has no Fargate spec at all" — which is best left to surface asServiceNotFoundExceptionvia the natural AWS error path. Consistent with the "let AWS errors bubble" stance.Plus: closes a coverage gap in
ExecutesWebStep. LPX-604 (#29) added headless-mode gating for solo DNS (SyncSoloRecordSetStep) but missed the multitenancy mirror.SyncMultitenancyRecordSetStepdereferences$this->config['apex']and['domain']unconditionally, so a headless multitenant deploy would undefined-key-crash before reaching ECS. Addingimplements ExecutesWebStepto the tenant step letsshouldBeRunning()skip it via the existing trait.README's
yolo init && yolo build && yolo sync production && yolo deploy productionwas nonsensical (yolo builderrored on the missing env arg; explicit build was redundant). Updated to:Is there anything the reviewer needs to know to deploy this?
yolo deploynow triggers the full build pipeline (rsync → composer/npm → docker build → ECR push) before the deploy steps. ~2-5 min added depending on cache hits.yolo buildandyolo deploy --app-version=<tag>as separate stages can now collapse to a singleyolo deploy production --app-version=<release-tag>call. If the split is still wanted for any reason (e.g. preview before promotion),yolo buildalone still works.Command::execute()which runsregisterAwsServices()and shouldBeRunning checks, hard to mock cleanly. Smoke-tested locally via the CL Fargate canary path.🤖 Generated with Claude Code