fix(cli): give veryfront open a way to reach the deployed site - #3605
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe ChangesDeployed site opening
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06d57e1336
ℹ️ 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".
`open` only ever built Cloud dashboard addresses, so nothing in the CLI could produce the environment URL Deploy prints. A reader who did not record that URL had no command to get it back, and `open --json` handed automation a dashboard link instead. `veryfront open --site` now opens the deployed environment — the canonical `https://<slug>.<environment>.veryfront.com` address `deploy` falls back to — defaulting to `production` and honoring `--env`. Without `--site`, `open` keeps opening dashboard pages exactly as before, and `--help` now says which is which. The deploy docs point readers at `--site` when they need the environment URL again, next to the existing note that plain `open` is the dashboard shortcut.
Review follow-up. When an environment carries a custom domain, Deploy prints `environment.domains[0]`, while `--site` always synthesizes the canonical `veryfront.com` hostname — `open` has no API token with which to read the configured domains. Both reach the same deployment, but the origins differ, so the guide no longer says `--site` prints the same URL and now tells automation that needs the custom domain to record what Deploy printed.
774049e to
57940ee
Compare
|
Rebased onto Every file this PR touches had also moved on
Docs.
Behaviour after the rebase, all five paths: Locally green: The P2 review thread about promising the exact Deploy URL was already answered by the second commit (the guide now says |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/open/command.ts`:
- Around line 37-44: Update buildSiteUrl to validate both projectSlug and
environment as canonical hostname labels before interpolating them into the URL
authority, reusing existing validators where available; reject invalid values
rather than constructing a non-Veryfront origin. Add regression coverage for
slash, query, and fragment characters in both inputs.
In `@docs/guides/deploying.md`:
- Around line 107-109: Update both site-opening command examples in the
deploying guide to include the --env staging selector, and update the
corresponding verification statement to explicitly verify the staging
environment rather than the default production environment.
🪄 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: e4a1c4ed-94e4-4b5d-bf28-1b0f1091a08b
📒 Files selected for processing (7)
cli/commands/open/command-help.tscli/commands/open/command.tscli/commands/open/handler.test.tsdocs/api-reference/veryfront/cli.mddocs/getting-started/deploy-project.mddocs/guides/deploying.mdtests/docs/guide-content.test.ts
`--site` is the only `open` path that puts a resolved value in the URL authority rather than its path. A slug or environment carrying `/`, `?`, or `#` pushed the hard-coded suffix into the path and produced a link Veryfront does not own: `evil.example/x` built `https://evil.example/x.production.veryfront.com`, which `open` then handed to the browser or printed as the `--json` result. The slug is not always typed at the prompt. It also comes from `veryfront.json` and the local project link, which arrive with a cloned repository, so it is validated rather than trusted. Both labels now have to be DNS labels, the same guard `push` applies before building a preview hostname, and fail with the existing INVALID_ARGUMENT usage error. Dashboard URLs are unchanged: the slug sits in the path there, where it cannot move the origin.
The examples sit under "For an existing nonproduction environment named `staging`", but a bare `open --site` targets `production`, so a reader copying the sequence would land on the wrong environment. Both examples now pass `--env staging`, and the sentence still states the default. The "After `veryfront deploy`" checklist keeps the generic `https://<slug>.<environment>.veryfront.com` form; it covers any environment, not the staging walkthrough.
kwakayama
left a comment
There was a problem hiding this comment.
Findings
- Medium —
--siterejects environment names the existing CLI/API contract accepts.cli/commands/open/command.ts:67requires--envto be a DNS label, soveryfront open --site --env "my env"exits withINVALID_ARGUMENT. However deploy accepts any non-empty environment string (cli/commands/deploy/command.ts:36), the API schema preserves arbitrary environment names (src/platform/adapters/veryfront-api-client/schemas/api.schema.ts:118), and the CLI explicitly tests"my env"as a valid environment reference (cli/commands/pull/command.test.ts:301). This leaves deployments to such environments—especially those with a configured custom domain that Deploy can return—unopenable through the newly documented command, contrary todocs/guides/deploying.md:107and:179. Resolve the environment/domain through the control plane when possible, or consistently restrict and document the supported environment-name contract; add coverage for a non-DNS environment name.
Rubric: correctness 32/40, tests 15/20, reliability/security 15/15, maintainability 12/15, scope/docs 8/10.
Review-Gate:
Reviewer: Codex
Reviewed-SHA: 7f02e33
Score: 82/100
Actionable-Findings: 1
Verdict: REQUEST_CHANGES
Symptom
veryfront openresolves only Cloud dashboard URLs. There is no way - flag,subcommand, or otherwise - to get the URL of the deployed site.
Reproduced against the published CLI (npm
veryfront@0.1.1229installed into asandbox prefix, read-only against pre-existing projects):
None is
https://vf-quickstart-rc.production.veryfront.com, andveryfront open --helplists no flag that would produce it.Why the previous fix did not settle this
#3576 (in the 0.1.1229 cut) read the symptom as two problems and fixed one and
a half of them:
openexited 1 with "No project found." in a freshly linked directory - itnever read
.veryfront/project.json. Genuinely fixed, and still fixed.openreaches the deployed site. fix(cli): resolve the local project link in veryfront open #3576 correctedthat claim in
veryfront-code/docs/**and test-locked it intests/docs/guide-content.test.ts.What it did not do was give the CLI the capability the docs had been promising.
It resolved the contradiction by retracting the promise: "record the URL Deploy
printed." That leaves a reader who did not record it with no command at all, and
leaves
open --jsonhanding automation a dashboard link.The doc half also never reached readers. The published site serves
veryfront-docs/docs/code/**, generated fromveryfront-code/docs/**by theauto/update-referencesync workflow - a PR a human merges. It has not landedsince #3576, so veryfront.com still shows the pre-#3576 text. That is a delivery
gap, not a diagnosis error, and re-editing
veryfront-code/docswould havereproduced it exactly. It is fixed at the site in veryfront/veryfront-docs#372.
Change
veryfront open --siteopens the deployed environment instead of a dashboardpage:
--site->https://<slug>.production.veryfront.com--site --env staging->https://<slug>.staging.veryfront.com--site --json-> the same URL in the success envelopeThat is the canonical address
deployitself falls back to(
buildEnvironmentUrlincli/shared/deployment/deploy-project.ts) when anenvironment carries no custom domain.
openhas no API token and cannot read anenvironment's configured domains, so
--sitealways builds the canonical form.An environment with a custom domain is reachable at both, but the origins
differ - the guide says so, and tells automation that needs the custom domain to
record the URL Deploy printed rather than rebuild it from
open --site.Without
--sitenothing changes:open,open --env <name>, andopen --studiobuild the same dashboard URLs they did before.--helpnowstates which is which.
Verification
Original repro command, run against this branch:
The redirect is that project's environment protection, i.e. the host is a live
deployment served by the proxy, not a dashboard route.
The four new
buildUrl/parseOpenArgscases incli/commands/open/handler.test.tswere written first and failed on theunchanged code (
siteparsed asundefined, dashboard URL returned).Docs
docs/guides/deploying.mdanddocs/getting-started/deploy-project.mdnow name--siteas the way back to the deployed environment, next to the existing notethat plain
openis the dashboard shortcut.tests/docs/guide-content.test.tslocks that in, alongside #3576's existing cases.
Live URLs to check after the docs sync
Once this ships and
auto/update-referenceruns, these pages must mentionveryfront open --site:Until then they must not - no released CLI accepts the flag. That is why
veryfront/veryfront-docs#372 corrects the false claim without introducing
--site.CI note
coverage shard 4/8failed twice withPromise resolution is still pending but the event loop has already resolvedafter the shard reported374 passed (3363 steps) | 0 failed- a post-run leak in the shard that carriessrc/transforms/esm/http-cache.test.ts, surfacing as the usual three red checks(shard,
coverage gate,tests (unit)). No assertion failed and no file inthat shard is touched here. Green on re-run without changes.
Summary by CodeRabbit
New Features
open --siteto open the deployed site directly.Documentation
Tests