fix(cli): classify a stale project link instead of unknown-error - #4532
Conversation
Switching accounts leaves `.veryfront/project.json` pointing at a project the new token cannot read. The lookup 404s, and both deploy adapters turned that into a bare `new Error(...)`, which the CLI boundary wrapped as `[unknown-error]` on the push path and re-wrapped as `[deployment-error]` on `up`. Neither told the user what to do, and the two adapters named the same project by different identifiers: deploy printed the configured slug while push printed the uuid the lookup actually used. Register `project-link-stale` (DEPLOY, 404, exit code 1) and raise it from both adapters through one shared wording helper, so `up` and `push` print the same sentence. `ProjectReferenceNotFoundError` now carries both identifiers the directory held, not just the one the lookup used, so the detail can name the project the way the user does and still say which uuid the link records. Auto-creating or prompting on a mismatch stays out of scope, as the reporter asked: `ProjectLink` records no owning account, so a deleted project and one owned by another account are indistinguishable from a 404. Refs veryfront/veryfront-issue-inbox#1445
Gate the classified error on the reference source. Deploy classified every ProjectReferenceNotFoundError as project-link-stale, but its suggestion names .veryfront/project.json, which only the local-link source comes from. A `--project <typo>` was told to delete this directory's correct link and run `veryfront up`, forking a duplicate project; a fresh clone with no link was pointed at a file that does not exist. Push had the same defect more narrowly: `byId` also covers VERYFRONT_PROJECT_ID and TENANT_PROJECT_ID. Every other source keeps the generic, source-neutral message it had before. Also drop the sentence the suggestion duplicated verbatim from the detail. Refs veryfront/veryfront-issue-inbox#1445 Follow-up: veryfront/veryfront-issue-inbox#1551
|
Warning Review limit reachedNext included review available in 42 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
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 |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Independent review: 🟡 minor fixes firstRisk if merged as-is: low | Reviewer confidence: high Look at this firstThe Blocking
Non-blocking
Where the reviewers disagreedFour real conflicts; I checked each against origin/main and ruled.
Where all three agreed and I confirmed: the 'byte-identical detail' claim is false at the command boundary, and the scope split is authorised by the genuine reporter's comment 2 rather than by the automated triage comment wearing @kojiwakayama's identity. All three lenses correctly refused to treat that triage comment as acceptance authority. On the pipeline's fail-first question: two lenses said 'yes', one said 'could-not-check'. I did not execute the suite either, but origin/main raises a plain Provenance: independent automated review (Claude Code) via @kojiwakayama's token. Tooling output, not a human approval. Three reviewers examined this diff through separate lenses (correctness, security/authz, contract & test quality); a synthesis pass adjudicated their disagreements against |
PROJECT_LINK_STALE told every affected user to "remove .veryfront/project.json and run veryfront up". push raises the same error, and `veryfront up` is not a re-link: upCommand resolves or creates the project and then calls deployProject.execute with publish: "live-source", so a push or CI user who follows the remedy performs a live Preview deployment they never asked for. origin/main's push message prescribed no command at all, so this was also a regression in guidance. ErrorCreateOptions carries no suggestion override, so one static string has to serve every raise site. The remedy now keeps only what is true at all of them: unlink, and let veryfront push create and link a project for the signed-in account. push passes createMissingReference: true, and with the link gone the source falls back to `inferred`, which shouldPersistProjectLink re-links. The push regression test now asserts the rendered remedy names veryfront push and does not name veryfront up. Verified fail-first: with the old suggestion restored the test fails on that assertion. Also, two evidence fixes the audit was right about: - The "byte-identical detail" comment in deploy-project.test.ts was false at the boundary the user sees. describeUpFailure re-wraps the shared sentence as `Preview publish failed: <sentence>` before `up` prints it, so byte-equality holds at the DeployProject adapter only. The comment now says that, and the PR body's matching claim is retracted. - The `--project` negative test rested entirely on its instanceof assertion: with the source gate removed, the classified detail still contains `Project "typo-slug" was not found.` and still never contains .veryfront/project.json. It now also asserts the generic message's own second sentence, which the classified detail does not contain. Verified by mutation with the instanceof assertion deleted as well: the test still fails. Refs veryfront/veryfront-issue-inbox#1445 Follow-up: veryfront/veryfront-issue-inbox#1551
|
Audit follow-up pushed as a89f9d0. Both blocking items were reproduced before being touched; one non-blocking item was taken as well, and one was declined. Fixed — the remedy prescribed a deploy to
|
The remedy on PROJECT_LINK_STALE no longer names `veryfront up`, but the source-gate comment in deploy-project.ts and the negative test's comment still described it that way. The hazard they explain is unchanged -- a `--project <typo>` user told to delete this directory's correct link forks a duplicate project -- only the command the suggestion names was stale. Refs veryfront/veryfront-issue-inbox#1445
|
State of this PR for a reviewerHead What this round changedComment- and prose-only; no behaviour, no assertions touched.
Claims in the body I re-verified against the tree, rather than trusting
One CI failure that was not this branchThe first run on this head failed Deliberately deferred, and where it is tracked
What I need from you
|
Review score: 94/100\n\nReviewed head: \n\nThe change cleanly separates stale local-link classification from ordinary project-reference failures, preserves project identity and source context, and prevents a stale-link error from prescribing to push users. The error registry, CLI paths, docs, and focused regressions are aligned.\n\nValidation:\n- 10 files changed, with focused push, deployment, and project-resolution coverage\n- CI, coverage, Sonar, CodeQL, lint, format, and typecheck pass\n- No unresolved review threads remain\n- The PR is still draft, so the Automated review gate is waiting on readiness\n\nScore is 94/100. Promote to ready for review so the final exact-head gate can run. |
Review score correction: 94/100Reviewed head: The change separates stale local-link classification from ordinary project-reference failures, preserves project identity and source context, and prevents a stale-link error from prescribing Validation:
The PR scores 94/100. Promote to ready for review so the final exact-head gate can run. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |



What was broken
Switching accounts leaves
.veryfront/project.jsonpinned to the previous account's project. The lookup 404s, and both commands raised a bareErrorthat reached the CLI boundary unclassified as[unknown-error]:cli/shared/deployment/deploy-project.ts:368—Project "..." was not found. Check the project reference...cli/commands/push/command.ts:1238—Project "..." was not found. Check .veryfront/project.json...The two also named the same project by different identifiers:
upreported the slug,pushreported the uuid, becauseProjectReferenceNotFoundErroronly carried whichever identifier the lookup happened to use (cli/shared/project-resolution.ts:77-86onmain).What this changes
PROJECT_LINK_STALE(src/errors/error-registry/deploy.ts), status 404, exit code 1, title "Linked project not found".ProjectReferenceNotFoundErrornow carries both identifiers the directory held, not just the one used for the lookup.describeStaleProjectReferencebuilds one sentence both adapters share:Project "slug" (uuid) was not found. The reference came from .veryfront/project.json; .... A reference that is only an id renders"x"once rather than"x" (x).upandpush"render byte-identical detail". That is false at the boundary the user sees, and it is retracted. The sentence is byte-identical at theDeployProjectadapter, which is the layercli/shared/deployment/deploy-project.test.tsasserts on;upthen re-wraps it asPreview publish failed: <sentence>indescribeUpFailure(cli/commands/up/command.ts:179-225) before it reaches the CLI. Sopushprints the bare sentence andupprints the prefixed one. What does hold end to end — and what the issue's bullet 4 asks for — is that both commands name the project by the same slug + uuid, name.veryfront/project.json, and carry the same[project-link-stale]slug and suggestion (describeUpFailurecopies slug/title/suggestion/exitCode for aVeryfrontError; covered bycli/commands/up/command.test.ts"keeps a registry error's classification when Preview publishing fails").Review finding addressed (major):
project-link-stalewas applied to every reference sourceThe reviewer was right, and I confirmed the failure before fixing it.
Deploy's reference can come from any of the seven sources in
cli/shared/config.ts:107-114, butPROJECT_LINK_STALE's title and suggestion are local-link-specific — the suggestion says to remove.veryfront/project.json. The first version classified all of them. Two real failures:veryfront deploy --project <typo>(sourceargument) told the user to delete this directory's correct link and re-create the project, which forks a duplicate — precisely the hazard the reporter flagged in comment 2.veryfront deployin a fresh clone (sourceinferred, no link at all) said "Linked project not found" and pointed at a file that does not exist.Push had the same defect, narrower: its
error.byIdguard also matchesVERYFRONT_PROJECT_IDandTENANT_PROJECT_ID, which resolve by id (cli/shared/config.ts:733-739) and do not come from the link file.Both sites now gate on
error.source.kind === "local-link"and fall through to the generic, source-neutral message every other source had before this branch. Since.veryfront/project.jsonrequires a non-emptyprojectId(cli/shared/project-link.ts:92), local-link always impliesbyId, so this is a strict narrowing with no behaviour change for the case the issue reported.Regression tests
Four tests, all confirmed to fail without the corresponding source change.
Positive (the issue's case) — these fail on
mainwith[unknown-error]:cli/commands/push/command.test.ts— "classifies a project link this account cannot see as project-link-stale"cli/shared/deployment/deploy-project.test.ts— same name, theup/deploy twinNegative (the review finding) — added this round:
cli/shared/deployment/deploy-project.test.ts— "does not blame the local link for a--projectreference that 404s"cli/commands/push/command.test.ts— "does not blame the local link for aVERYFRONT_PROJECT_IDthat 404s"I verified the two negative tests fail without the gate by temporarily replacing the
error.source.kind === "local-link"condition withtruein both files and re-running:The gate was then restored and both pass.
Gates run
deno task test:file cli/shared/deployment/deploy-project.test.tsdeno task test:file cli/commands/push/command.test.tsdeno task test:file cli/shared/project-resolution.test.tsdeno task test:file src/errors/error-registry.test.tsdeno check --no-lockon all changed.ts(incl. tests)deno fmt --checkon all changed filesdeno task lintdeno task lint:test-typecheckdeno task lint:testing-front-doorwithMockFetch, no bareglobalThis.fetch)deno task lint:cli-boundarydeno task lint:anti-slopdeno task lint:test-semantic-dispositionsdeno task docs+deno task docs:errorsdeno task docs:api-reference:checkdeno task docs:errors:checkdeno task test:fileruns--no-check, sodeno checkwas run separately on every changed file per the repo's CI-gate guidance.Re-run after the audit fixes, on the changed files only:
deno task test:file cli/commands/push/command.test.tsdeno task test:file cli/shared/deployment/deploy-project.test.tsdeno check --no-lockon the three changed.tsfilesdeno fmt --checkon the four changed filesdeno task docs:errors+docs:errors:checkdeno task lint:testing-front-doordeno task lint:anti-slopdeno task lint:test-semantic-dispositionsPre-existing failure, not from this branch:
deno task docs:validatefails ondocs/api-reference/veryfront/agent.md:2004(createManagedBrokerHandler, a JSDoc starting with "Handle "). That file is untouched here; I confirmed it by stashing this branch's changes and re-runningscripts/docs/validate-api-reference.tson the pristine tree, which reports the identical single issue.I also kept the generated
docs/api-reference/veryfront/errors.mddiff to one line: an earlier wording of thePROJECT_LINK_STALEdoc comment exceeded the table's 362-char description column and realigned all 150 rows. The comment was shortened to fit the existing width.Audit findings addressed (this round)
Blocking — the remedy string prescribed a deploy to
pushusers. Fixed insrc/errors/error-registry/deploy.ts. The suggestion said "remove .veryfront/project.json and run veryfront up to create and link a project for this account". Bothpush(cli/commands/push/command.ts:1246) and the deploy adapter raise this error, andveryfront upis not a re-link:upCommandresolves or creates the project itself and then callsdeployProject.executewithpublish: "live-source"(cli/commands/up/command.ts:371), i.e. it publishes a live Preview deployment. Apushor CI user following the remedy performs a deploy they never asked for — and that is a regression in guidance versusmain, whose push message only said "remove it to let Veryfront create a project for this directory".ErrorCreateOptions(src/errors/types.ts) has nosuggestionoverride, so one static string has to serve every raise site; per-command remedies would mean widening the error-framework contract, which is the same reason the "name the signed-in account" finding was declined below.The remedy therefore drops the command it cannot know and keeps only what is true everywhere: unlink, and letCorrection (record accuracy): that sentence misdescribes what shipped and is retracted. The shipped suggestion does name a command —pushre-create.veryfront push— it only dropsveryfront up. What it avoids is naming the command that would turn a re-link into a deploy; namingveryfront pushis true at every raise site, because the remedy at both sites is the same unlink, andpushcompletes it on its own. I verified that half —pushpassescreateMissingReference: true(cli/commands/push/command.ts:1231), and with the link gone the source falls back toinferred, whichshouldPersistProjectLink(cli/shared/project-resolution.ts:142-144) re-links. New wording:cli/commands/push/command.test.tsnow asserts the rendered remedy namesveryfront pushand does not nameveryfront up. Verified fail-first: with the old suggestion restored, that test fails (13 passed | 1 failed).Blocking — the false "byte-identical" claim. Retracted above, and the stale comment in
cli/shared/deployment/deploy-project.test.tsnow says what the assertion actually proves (the adapter boundary) and namesdescribeUpFailure's prefix.Non-blocking, taken anyway — a near-vacuous negative test. The audit was right that "does not blame the local link for a
--projectreference that 404s" rested entirely on its oneinstanceof VeryfrontError === falseassertion: with the source gate removed, the classified detail still containsProject "typo-slug" was not found.and still never contains.veryfront/project.json. It now also asserts the generic message's own second sentence,Check the project reference or remove it to let deploy create a project, which the classified detail does not contain. Verified by mutation — gate replaced withtrueand theinstanceofassertion deleted, the test still fails, on exactly that new assertion:The
instanceofassertion is kept. One review lens wanted it dropped; dropping it is exactly what would have made this test detect nothing.Not taken.
describeStaleProjectReference'sslug !== idcollapse branch is unreachable from today's two call sites (both gated onlocal-link, andisProjectLinkrequires both identifiers). It is real dead weight, but the fix is deleting behaviour plus its tests, which is not what a fix PR under review should do silently — recorded here instead.Review findings declined
Minor — "name the signed-in account instead of telling the user to run
veryfront whoami". Declined here, deferred to the follow-up issue. The registry suggestion is a static string andErrorCreateOptions(src/errors/types.ts:57-66) has nosuggestionoverride, so the account can only be named in the detail. Neither raise site holds the identity:cli/shared/deployment/deploy-project.tsandcli/commands/push/command.tshave an API token, not an email.updoes holduserInfofromensureAuthenticated, but it is not plumbed throughDeployProjectRequest, and push would need awhoamiround trip issued from inside an error path. Widening a shared error-framework contract and adding a network call on a failure path is not a cheap minor fix; it is recorded as a bullet on the follow-up.Other review findings applied
Minor — suggestion duplicated the detail verbatim. Fixed. The suggestion dropped its leading "The linked project was deleted, or it belongs to an account other than the one you are logged in as", which the detail already says, and now carries only the remedy.
Minor — deferred half of the issue untracked. Fixed. Filed veryfront/veryfront-issue-inbox#1551 covering
veryfront upauto-recovery/prompt, a first-classveryfront link <slug>, naming the signed-in account, and recording the owning account in the link so deleted-vs-not-visible can be told apart. It is referenced from the commit message and from thePROJECT_LINK_STALEdoc comment.Final accuracy pass (
8153c5d)Comment-only. When the remedy stopped naming
veryfront up, two comments were left quoting the old one: the source-gate comment incli/shared/deployment/deploy-project.tsand the comment on the negative test incli/shared/deployment/deploy-project.test.tsboth still saidproject-link-staletells the user to delete the link and runveryfront up. The hazard they explain is unchanged — a--project <typo>user told to delete this directory's correct link forks a duplicate project — only the command name was stale. No behaviour, no assertions touched.deno fmt --checkon both changed filesdeno check --no-lockon both changed filesdeno task test:file cli/shared/deployment/deploy-project.test.tsLine references in this body were also re-checked against the current tree and three stale ones corrected (
createMissingReferenceatpush/command.ts:1231,ErrorCreateOptionsatsrc/errors/types.ts:57-66, and themainconstructor atproject-resolution.ts:77-86).Scope note
This ships the error-classification + guidance half of #1445, which is the split the reporter recommended in comment 2. Issue bullets 3 (classify, no
unknown-error) and 4 (consistent slug+uuid reference naming.veryfront/project.json) are met. Bullet 2 (upshould recover) and theveryfront link <slug>remedy in bullet 1 are #1551 — they need a product decision between auto-create, prompt, and fail, which comment 2 leaves open.#1445 stays open on merge. This PR is
Refs, notCloses. The audit flagged that veryfront-issue-inbox#1551's body said "#1445 is being closed as the error-classification + guidance half only", which would invite someone to close an issue whose headline bullet (upshould recover) is untouched. That sentence in #1551 has been corrected.Refs veryfront/veryfront-issue-inbox#1445