fix(cli): make deploy honour --project instead of silently ignoring it - #3507
Conversation
`veryfront deploy --project X` resolved the project from the working directory
and dropped the flag. Worse, deploy does not merely promote: it pushes the
working directory first, so naming a project could upload an unrelated
directory into it and promote the result. The plumbing already existed --
`deploy-project.ts` handles `request.projectSlug` and labels it
`{ kind: "argument", name: "--project" }` -- only the CLI arg map was missing
the field, so `createArgParser` discarded it.
Naming a project now also makes the deploy promote-only, which is what stops
the upload: a directory is unrelated to a project the caller named by slug.
That made dry run and apply disagree, which matters because the reported
incident was a dry run. The dry-run branch validated the push receipt only for
an `ensure-pushed` source, so a named project skipped the check and reported a
deploy the identical apply refused. It now validates for a named project too.
`--skip-source-push` keeps its existing behaviour; that caller has said the
source is handled.
The regression test names a project the directory did NOT push, which is the
incident. Checked against the mutation that matters -- dropping the projectSlug
forwarding -- and it fails, where a test using the already-linked slug passes
either way.
📝 WalkthroughWalkthroughThe deploy command now accepts ChangesNamed project deployment
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as deploy command
participant DP as deploy-project
participant PR as pushed-source validation
participant DE as deployment executor
CLI->>DP: submit projectSlug with already-pushed source mode
DP->>PR: validate matching verified push receipt
PR-->>DP: return receipt validation result
alt receipt matches
DP->>DE: create deployment
DE-->>CLI: return deployment result
else receipt missing or mismatched
DP-->>CLI: return receipt error without upload or deployment
end
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 `@cli/commands/deploy/command-help.ts`:
- Line 51: Update the help text string in the deploy command’s project option
description to replace “this directory” with “the selected project directory,”
accurately reflecting the directory used for receipt validation when --dir is
provided.
In `@cli/commands/deploy/command.integration.test.ts`:
- Around line 640-676: Update the test around deployCommand to add an
other-project fixture with its own project ID and environment so
createDeployFetchHandler resolves the target before receipt validation. Assert
the specific receipt-project mismatch error rather than any rejection, and run
equivalent assertions for both dryRun: false and dryRun: true while preserving
the no-upload verification.
In `@cli/commands/deploy/command.test.ts`:
- Line 291: Replace the hard-coded absolute projectDir value in the affected
deployment test with a relative placeholder path, preserving the existing fake
deployment executor behavior.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5c6ee2c8-f025-4cd5-8fe0-4b34550f94c6
📒 Files selected for processing (5)
cli/commands/deploy/command-help.tscli/commands/deploy/command.integration.test.tscli/commands/deploy/command.test.tscli/commands/deploy/command.tscli/shared/deployment/deploy-project.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d49d2734de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The mismatch test named a project the fixture could not resolve, so the deploy could fail on that lookup and the assertion -- which only required some error -- would not have noticed. It now resolves `other-project`, leaving the receipt as the only thing that can refuse, and asserts the refusal by message. Also drops a hard-coded absolute path from the fake-executor test, which AGENTS.md prohibits, and corrects the help note: with `--dir` the receipt is read from the selected project directory rather than the working one.
The first attempt at this test passed with the fix reverted, so it was deleted and the gap written into the PR description instead. The cause was the fixture, not the idea: `other-project` did not resolve, so the dry run failed on that lookup whether or not it validated the receipt. Resolving that project for the mismatch test removed the blocker, so the case can now be written honestly. Reverting the dry-run condition makes it fail -- the dry run stops validating, reports a deploy, and the assertion notices.
veryfront deploy --project Xaccepted the flag and threw it away, resolving the project from the working directory instead:Worse than a no-op.
deploydoes not merely promote — it pushes the working directory as project source first. Combined with the ignored flag, one command can upload an unrelated directory into a project and promote the result. This nearly put a framework repository live on a customer's site; only the dry-run's project name caught it.Cause
The plumbing already existed:
cli/shared/deployment/deploy-project.ts:226-237handlesrequest.projectSlugand labels it{ kind: "argument", name: "--project" }. OnlyparseDeployArgs' map was missing the field, socreateArgParser— which copies only mapped fields — discarded it.Naming a project now also makes the deploy promote-only. That is what stops the upload: a working directory is unrelated to a project the caller named by slug.
Dry run and apply now agree
Making the source
already-pushedexposed a second problem, and it matters because the reported incident was a dry run. The dry-run branch validated the push receipt only for anensure-pushedsource, so a named project skipped the check entirely and reported a deploy that the byte-identical apply refused.It now validates for a named project too.
--skip-source-pushkeeps its existing dry-run behaviour — that caller has explicitly said the source is handled — so this does not tighten an unrelated path.Test
refuses to deploy a project this directory did not pushwrites a valid receipt formy-projectand then deploys--project other-project, so nothing but the mismatch can stop it, and nothing may be uploaded on the way.Checked against the mutation that matters — dropping the
projectSlugforwarding — and it fails. A test using the already-linked slug passes either way, which is why this one names a different project.Both modes are covered
refuses to deploy a project this directory did not pushwrites a valid receipt formy-project, resolvesother-projectso nothing but the mismatch can refuse, and asserts the refusal by message.refuses the same mismatch in a dry run as in an applypins the two modes together.Each was checked in the direction that matters:
projectSlugforwardingThe dry-run case took two attempts. The first passed with the fix reverted and was deleted rather than shipped; the cause was the fixture resolving only one project, so the dry run died on a lookup either way. Resolving
other-projectfor the mismatch test removed that blocker and the case could then be written honestly.Found via veryfront-issue-inbox#430. The promote-only behaviour change is worth a reviewer's attention: anyone who relied on
deploy --project Xbootstrapping the first push must now runveryfront pushfirst.Summary by CodeRabbit
New Features
--project/-p.--diroption for specifying the deployment directory.Bug Fixes