Repository navigation
fix(init): don't recreate projects linked without git - #8646
domitriusclark wants to merge 3 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. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: 🔵 Low · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description clearly explains the linked-project initialization bug, the implementation, test coverage, reported validation, risk, and rollback plan. It directly matches the changeset. |
| Title check | |
The title clearly identifies the main change: preventing duplicate project creation when an existing project is 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 4… |
| 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 4 files.
✨ 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
Some tools did not complete. Review the errors below.
🔧 ESLint
-
If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.
src/commands/init/init.ts
-
There are suppressions left that do not occur anymore. Consider re-running the command with
--prune-suppressions.
src/utils/get-repo-data.ts
-
ESLint skipped: the matched ESLint configuration already failed (unknown).
tests/integration/commands/init/init.test.ts
-
ESLint skipped: the matched ESLint configuration already failed (unknown).
-
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.
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.
| ) | ||
| } | ||
| 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.
| } | ||
| 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.
|
|
||
| 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.
| } | ||
|
|
||
| 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.
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.