Repository navigation
fix(init): don't recreate projects linked without git - #8646
domitriusclark wants to merge 4 commits into
Conversation
Rerunning init in a folder linked to a site with no repo and no git remote created a second site. Treat it as done, and ignore a linked site that returns 404 so the normal create/link flow runs.
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description clearly explains the init behavior change, linked projects without Git remotes, stale project handling, tests, risk, and rollback. |
| Title check | |
The title clearly identifies the main change: preventing project recreation when init reruns for a project linked without Git. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5 files. (1 skipped: 1 unsupported.)
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Linked repositories: Some configured repositories could not be included in this review. Cross-repository analysis may be incomplete. Please check linked-repository access and settings.
Comment @coderabbitai help to get the list of available commands.
📊 Benchmark resultsComparing with f1632d5
|
commit: |
| return (await api.getSite({ siteId: siteInfo.id })) as unknown as SiteInfo | ||
| } catch (error_) { | ||
| if ((error_ as APIError).status === 404) { | ||
| warn(`The Netlify project ${siteInfo.id} linked to this folder no longer exists. Continuing without it.`) |
There was a problem hiding this comment.
[blocking] After this warning, command.netlify.siteInfo and the linked siteId still point at the deleted project, so the rest of init doesn't actually continue without it.
With a git remote, picking "Connect this directory to an existing Netlify project" lands in link(), which sees a non-empty siteInfo, prints Project already linked to "undefined" and hands back the deleted { id }. init saves that id again and runs configureRepo against it. Creating a new project gets through, but the link() call inside sitesCreate prints the same already linked to "undefined" line. The stale-id test hits that too, it just doesn't check for it.
There was a problem hiding this comment.
Fixed in 3a23cde. On a 403 or 404, init now removes siteId from .netlify/state.json, clears command.siteId and resets siteInfo to {}, so link() and sitesCreate see an unlinked folder. The stale-id tests now assert no "already linked" line and that the saved id is the new project's. A new test covers a git remote plus create-new; it PATCHes only the new site.
The connect-existing test can't drive link()'s prompt without a TTY, so it stops at the non-interactive link error. It still shows there is no "already linked" line and that siteId is cleared. On the previous commit it hangs.
On a 403 this deletes siteId from a state.json a teammate may have committed. Creating or linking a project overwrites that value anyway.
| } | ||
| return undefined | ||
| } | ||
| throw error_ |
There was a problem hiding this comment.
[consider] Anything other than a 404 here now ends init with the raw API error, where before it carried on with the bare { id }. Intended? It's not in the description and no test reaches it.
There was a problem hiding this comment.
Intended, and changed in c1211a7. A 403 now continues like a 404, with its own warning. Any other error stops init with the project id and status rather than continuing, so a transient 5xx can't lead to a duplicate create. The 500 test checks the error and that nothing is created.
| // Look for local repo | ||
| const repoData = await getRepoData({ workingDir: command.workingDir, remoteName: options.gitRemoteName }) | ||
| if ('error' in repoData) { | ||
| if (linkedSiteInfo) { |
There was a problem hiding this comment.
[consider] Every getRepoData error ends up here, not only a missing remote. A --git-remote-name that doesn't exist, or a remote URL that won't parse, gets "already linked, run deploy" with nothing about the remote.
There was a problem hiding this comment.
Fixed in 3a23cde. If the remote can't be read, a linked folder now prints the remote error and how to fix it before the "already linked" message. A new test uses --git-remote-name upstream.
I kept this exit rather than main's path, because on main these errors reach the "create without git" prompt, which is how the duplicate happens. This also fixed a no-git folder reporting "no gitconfig to be found": getRepoData now looks for .git before reading the git config.
| t.expect(second.stdout).not.toContain(NO_GIT_PROMPT) | ||
| t.expect(second.stdout).toContain('This folder is already linked to the Netlify project test-site-name') | ||
| t.expect(second.stdout).toContain('https://test-site-name.netlify.app/') | ||
| t.expect(second.stdout).toContain('deploy') |
There was a problem hiding this comment.
[nit] Almost any output contains deploy. customizeExitMessage on the new exit path isn't covered either.
There was a problem hiding this comment.
Fixed in 3a23cde. The rerun test now asserts the full deploy line and the unlink hint.
I removed customizeExitMessage from this exit rather than test it. Its only caller, netlify dev, calls init only when no project is linked, so it can't reach this path. I'll restore it with a test if you'd rather keep it.
A 403 now continues like a 404. Other errors stop init with the project id and status instead of risking a duplicate create on a transient failure. The linked-folder message also says how to switch projects.
A 403 or 404 linked project is now removed from state and memory, so link and create treat the folder as unlinked. A linked folder whose git remote can't be read reports the remote error instead of only deploy.
seancdavis
left a comment
seancdavis
left a comment
There was a problem hiding this comment.
Some comments for consideration. Take a look and I'll approve if you don't want to adjust anything.
|
|
||
| const logLinkedWithoutGitAndExit = ({ siteInfo, repoError }: { siteInfo: SiteInfo; repoError: string }): never => { | ||
| if (repoError !== NO_GIT_REMOTE_ERROR) { | ||
| warn(`Could not read the git remote: ${repoError}`) |
There was a problem hiding this comment.
[consider] When the remote URL doesn't parse, repoError carries the whole URL from .git/config, so any credentials in it land in the terminal or a CI log. Main never printed this error.
There was a problem hiding this comment.
Fixed in 6780088. getRepoData now strips anything before @ in the URL, so the error shows https://***@.... The fix is in getRepoData itself, so any caller that prints the error gets it too. A unit test covers a URL with a password.
| ) | ||
| } | ||
| warn(unusableLinkWarning) | ||
| command.netlify.state.delete('siteId') |
There was a problem hiding this comment.
[consider] On a 403 this removes the saved link before anything's been asked, so Ctrl+C at the next prompt leaves a teammate's committed siteId gone.
There was a problem hiding this comment.
Fixed in 6780088. init no longer deletes siteId from state.json. It ignores the linked project for this run only, and the file changes only when a new project is linked or created. Ctrl+C now leaves the teammate's siteId in place. The connect-existing test now checks that an aborted run keeps the old id.
| } | ||
| warn(unusableLinkWarning) | ||
| command.netlify.state.delete('siteId') | ||
| command.siteId = undefined |
There was a problem hiding this comment.
[consider] The unlink command sends sites_unlinked when it drops siteId. This path doesn't, so these unlinks won't show up there.
There was a problem hiding this comment.
Fixed in 6780088. init now sends sites_unlinked with kind: 'init' and reason: 'forbidden' or 'not_found' when it stops using the link. It's sent when init drops the link for the run, so an aborted run still counts, even though state.json keeps the id.
|
|
||
| test('netlify init ignores a linked project that no longer exists', async (t) => { | ||
| await withSiteBuilder(t, async (builder) => { | ||
| await builder.withStateFile({ siteId: 'deleted_site_id' }).build() |
There was a problem hiding this comment.
[nit] These 404s come from the mock API's fallback for an unknown route, not a declared response like the 403 and 500 tests have.
There was a problem hiding this comment.
Fixed in 6780088. The deleted-project tests now declare sites/deleted_site_id as a 404 route, the same way the 403 and 500 tests do.
| } | ||
|
|
||
| const logLinkedWithoutGitAndExit = ({ siteInfo, repoError }: { siteInfo: SiteInfo; repoError: string }): never => { | ||
| if (repoError !== NO_GIT_REMOTE_ERROR) { |
There was a problem hiding this comment.
[nit] This tells "no remote" apart from the other errors by matching the message text.
There was a problem hiding this comment.
Fixed in 6780088. getRepoData errors now carry a reason (no-remote, unknown-remote, invalid-url or unreadable), and init checks reason !== 'no-remote' instead of the message.
An unusable linked project is now ignored for the run instead of deleted from state.json, so aborting keeps it, and init reports sites_unlinked. getRepoData returns a reason code and hides URL credentials in its error.
seancdavis
left a comment
seancdavis
left a comment
There was a problem hiding this comment.
The description still only covers the first commit. Worth a refresh before merge.
| } | ||
| warn(unusableLink.warning) | ||
| await track('sites_unlinked', { siteId: siteInfo.id, kind: 'init', reason: unusableLink.reason }) | ||
| ignoreLinkedSiteForThisRun(command) |
There was a problem hiding this comment.
[consider] When the dead id comes from NETLIFY_SITE_ID, this still says the folder's link is the problem, and picking create saves the new project over whatever state.json had.
LocalState.get('siteId') returns the env var ahead of state.json, so the next run resolves the dead id again and offers to create another one.
| `${netlifyCommand()} unlink`, | ||
| )}, then ${chalk.cyanBright.bold(`${netlifyCommand()} init`)}.`, | ||
| ) | ||
| return exit() |
There was a problem hiding this comment.
[nit] This exits 0 even when the remote couldn't be read, so an agent running init --git-remote-name upstream sees success and moves on without the repo connected.
| return exit() | ||
| } | ||
|
|
||
| const ignoreLinkedSiteForThisRun = (command: BaseCommand): void => { |
There was a problem hiding this comment.
[nit] This rebuilds BaseCommand's site object by hand, so anything added to site there later won't show up here. The id setter isn't called anywhere on these paths either.
Summary
Problem: Rerunning
netlify initin a folder linked to a project with no git remote asks "create a Netlify project without a git repository?" again, and answering yes creates a second project and relinks the folder. Agents reruninitoften, and projects started from netlify.ai usually have no git, so this hits the main setup path.Approach: In
init.ts, a folder linked with no git remote now prints the project name, URL andnetlify deployas the next step, then exits. A linked project with no repo still goes on to repo setup once a remote exists. If the linked project returns 404,initwarns and runs the normal create/link flow. It reloads teams first, because config falls back offline for a deleted project and leaves the team list empty. That neededBaseCommand.refreshAccountsmade public. Prompts for new folders are unchanged.Tests: three new integration tests in
tests/integration/commands/init/init.test.ts: rerun after a manual create (one POST to<account>/sites, no second no-git prompt), linked project plus new remote (reaches repo PATCH), stale project id (warning, one create).npm exec vitest -- run tests/integration/commands/init/init.test.ts: 12 passed. The rerun and stale-id tests fail on main. typecheck, lint, format:check and docs are clean, with no doc changes.Risk:
initnow makes one extragetSitecall when config resolved the site without details (offline fallback). Merges cleanly with #8638; the merged tree passes all 13 init tests.Rollback: revert this PR.