Skip to content

feat: add ai command for cli to start the project #7352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

denar90
Copy link

@denar90 denar90 commented Jun 16, 2025

🎉 Thanks for submitting a pull request! 🎉

Summary

Fixes #<replace_with_issue_number>


For us to review and ship your PR efficiently, please perform the following steps:

  • Open a bug/issue before writing your code 🧑‍💻. This ensures we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or something that`s on fire 🔥 (e.g. incident related), you can skip this step.
  • Read the contribution guidelines 📖. This ensures your code follows our style guide and
    passes our tests.
  • Update or add tests (if any source code was changed or added) 🧪
  • Update or add documentation (if features were changed or added) 📝
  • Make sure the status checks below are successful ✅

A picture of a cute animal (not mandatory, but encouraged)

Copy link

github-actions bot commented Jun 16, 2025

📊 Benchmark results

Comparing with 1348a3e

  • Dependency count: 1,081 (no change)
  • Package size: 288 MB ⬇️ 0.00% decrease vs. 1348a3e
  • Number of ts-expect-error directives: 399 (no change)

@denar90 denar90 changed the title Artemdenysov/wrfl 2615 add ai command for cli to start the project (feat): add ai command for cli to start the project Jun 16, 2025
@denar90 denar90 marked this pull request as ready for review June 23, 2025 08:44
@denar90 denar90 requested a review from a team as a code owner June 23, 2025 08:44
@@ -0,0 +1,447 @@
import { resolve } from 'node:path'
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

part of code here is duplication, but we still not setelled the command and want to see how it feels so then we can extract stuff into common utils after we work on making command fully public

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why trialing the new command should mean more code duplication? If you move the generic methods you're reusing to a separate file (which I would argue should already be the case), you're making it easier for this PR to be reviewed, you're going to have less code to maintain, and you reduce the number of places where you might need to adjust things if an issue or an improvement come up.

If you then decide that you want to keep the new command in its current shape, great — you already have the code in the structure you want and no further changes are needed; if you decide you want to bin it, you can just delete it and the code you abstracted away into a separate file still makes sense on its own.


// Step 4: Save AI instructions to file
if (projectInfo.prompt) {
const ntlContext = await fetch(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in future we don't wanna fetch context if we installing MCP

@@ -0,0 +1,447 @@
import { resolve } from 'node:path'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why trialing the new command should mean more code duplication? If you move the generic methods you're reusing to a separate file (which I would argue should already be the case), you're making it easier for this PR to be reviewed, you're going to have less code to maintain, and you reduce the number of places where you might need to adjust things if an issue or an improvement come up.

If you then decide that you want to keep the new command in its current shape, great — you already have the code in the structure you want and no further changes are needed; if you decide you want to bin it, you can just delete it and the code you abstracted away into a separate file still makes sense on its own.

}

// Generate MCP configuration for the detected IDE
const generateMcpConfig = (ide: ConsumerConfig): string => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you returning the config as a string here only to then re-parse it into an object in configureMcpForVSCode?

await fs.writeFile(configPath, JSON.stringify(updatedConfig, null, 2), 'utf-8')
log(`${chalk.green('✅')} VS Code MCP configuration saved to ${chalk.cyan('.vscode/mcp.json')}`)
} catch (error) {
throw new Error(`Failed to configure VS Code MCP: ${error instanceof Error ? error.message : 'Unknown error'}`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will error not be an instance of Error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's always unknown, but here yeah it will be Error, it's more following code pattern across repo

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a couple of instances of that, which doesn't feel like enough of a reason to perpetuate this and just duplicate this code every time we handle an error, especially when we know for a fact that the type will be Error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a normal idiom--TypeScript can't guarantee caught values are errors.


// VS Code specific MCP configuration
const configureMcpForVSCode = async (config: string, projectPath: string): Promise<void> => {
const configPath = resolve(projectPath, '.vscode', 'mcp.json')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Save the path to the .vscode directory in a variable to avoid resolving the path multiple times.


// Windsurf specific MCP configuration
const configureMcpForWindsurf = async (config: string, _projectPath: string): Promise<void> => {
const { homedir } = await import('node:os')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using a dynamic import here?

const { api } = command.netlify

log(`${chalk.blue('🤖 Initializing AI project')} with rules...`)
log(`${chalk.gray('Hash:')} ${hash}`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have to print the hash? Why do people care?

let mcpConfigured = false

if (detectedIDE) {
log(`${chalk.green('✅')} Detected IDE: ${chalk.cyan(detectedIDE.presentedName)}`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we settle on either "development environment" or "IDE"? Using both next to each other feels unnecessarily confusing.

} else {
log(chalk.yellowBright(`🔧 Step 2: Manual MCP Configuration`))
log(` ${chalk.cyan(detectedIDE.key)} detected - MCP setup was skipped`)
log(` ${chalk.gray('You can configure MCP manually later for enhanced AI capabilities')}`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How?

@denar90 denar90 requested a review from eduardoboucas June 23, 2025 10:48
Copy link
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving so you can get unblocked, but I left some notes and questions that I think we should be addressed.

log(`${chalk.green('✅')} AI instructions saved to ${chalk.cyan('AI-instructions.md')}`)
} catch (error) {
const errorMessage = error instanceof Error ? error.message : 'Unknown error'
log(`${chalk.yellow('⚠️')} Warning: Failed to save AI instructions: ${errorMessage}`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI has a warn method precisely for this purpose:

export const warn = (message = '') => {
const bang = chalk.yellow(BANG)
log(` ${bang} Warning: ${message}`)
}

* Generate MCP configuration for the detected IDE or development environment
*/
export const generateMcpConfig = (ide: ConsumerConfig): Record<string, unknown> => {
const configs: Record<string, Record<string, unknown>> = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to properly type this.

}
}

const fetchProjectInfo = async (url: string): Promise<ProjectInfo> => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to have a comment explaining what is the URL we're hitting — is it the Netlify API, something else?


const getRepoUrlFromProjectId = async (api: NetlifyAPI, projectId: string): Promise<string> => {
try {
const siteInfo = (await api.getSite({ siteId: projectId })) as SiteInfo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not already have the site information populated by the base command? Or is this a different site? If we could use that, we'd avoid making an extra API call.

{
method: 'GET',
headers: {
'Content-Type': 'text/plain',
Copy link
Member

@eduardoboucas eduardoboucas Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you setting Content-Type on a GET request? Did you mean to use Accept?

}

// Update working directory to cloned repo
process.chdir(targetDir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this?

@denar90
Copy link
Author

denar90 commented Jun 23, 2025

Awesome 🙌 thank you

@denar90 denar90 enabled auto-merge (squash) June 23, 2025 13:38
@denar90 denar90 added the automerge Add to Kodiak auto merge queue label Jun 23, 2025
@sarahetter sarahetter changed the title (feat): add ai command for cli to start the project feat: add ai command for cli to start the project Jun 23, 2025
@denar90 denar90 disabled auto-merge June 23, 2025 16:09
@denar90 denar90 removed the automerge Add to Kodiak auto merge queue label Jun 23, 2025
@denar90 denar90 marked this pull request as draft June 23, 2025 16:14
…5-add-ai-command-for-cli-to-start-the-project
@denar90 denar90 force-pushed the artemdenysov/wrfl-2615-add-ai-command-for-cli-to-start-the-project branch from c73cb02 to d50b455 Compare July 11, 2025 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants