Skip to content

Make CLI cleanup cross-platform #3620

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.

Sign up for GitHub

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

Open
dipeshbabu wants to merge 1 commit into
base: main
Choose a base branch
from
+14 −6
Open

Make CLI cleanup cross-platform #3620

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions packages/cli/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ program
.option('-y, --yes', 'Skip interactive prompts and use defaults')
.option('--no-pull', 'Skip pulling the latest Docker images')

async function runCommandQuiet(command: string[]): Promise<boolean> {
return new Promise((resolve) => {
const process = spawn(command[0], command.slice(1), { stdio: 'ignore' })
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 17, 2026

Choose a reason for hiding this comment

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

P2 Variable name shadows global process

Naming the spawned child process silently shadows Node.js's built-in process global for the rest of this function body. Any future code added inside this function that tries to call process.exit(), process.env, etc., will unexpectedly operate on the ChildProcess object instead. The same pattern already exists in runCommand (line 51) — this PR replicates it. Rename both to something like proc or child.

Suggested change
const process = spawn(command[0], command.slice(1), { stdio: 'ignore' })
const proc = spawn(command[0], command.slice(1), { stdio: 'ignore' })
proc.on('error', () => {
resolve(false)
})
proc.on('close', (code) => {
resolve(code === 0)
})

process.on('error', () => {
resolve(false)
})
process.on('close', (code) => {
resolve(code === 0)
})
})
}
Comment on lines +27 to +37
Copy link
Contributor

@greptile-apps greptile-apps bot Mar 17, 2026

Choose a reason for hiding this comment

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

P2 Code duplication with runCommand

runCommandQuiet is functionally identical to the existing runCommand (lines 49–59), differing only in the stdio option ('ignore' vs 'inherit'). Keeping two near-identical implementations will make future maintenance harder. Consider refactoring into a single function with an optional parameter:

async function runCommand(command: string[], quiet = false): Promise<boolean> {
  return new Promise((resolve) => {
    const proc = spawn(command[0], command.slice(1), { stdio: quiet ? 'ignore' : 'inherit' })
    proc.on('error', () => resolve(false))
    proc.on('close', (code) => resolve(code === 0))
  })
}

Then callers use runCommand([...], true) for quiet execution, removing the need for runCommandQuiet entirely.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link

@cursor cursor bot Mar 17, 2026

Choose a reason for hiding this comment

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

New function duplicates existing one with minor difference

Low Severity

runCommandQuiet is a near-exact copy of the existing runCommand — the only difference is stdio: 'ignore' vs stdio: 'inherit'. Instead of duplicating the entire function, runCommand could accept an optional quiet parameter (or a stdio option) to control output visibility, avoiding redundant logic.

Additional Locations (1)
Fix in Cursor Fix in Web


function isDockerRunning(): Promise<boolean> {
return new Promise((resolve) => {
const docker = spawn('docker', ['info'])
Expand Down Expand Up @@ -66,12 +78,8 @@ async function pullImage(image: string): Promise<boolean> {
}

async function stopAndRemoveContainer(name: string): Promise<void> {
try {
execSync(`docker stop ${name} 2>/dev/null || true`)
execSync(`docker rm ${name} 2>/dev/null || true`)
} catch (_error) {
// Ignore errors, container might not exist
}
await runCommandQuiet(['docker', 'stop', name])
await runCommandQuiet(['docker', 'rm', name])
}

async function cleanupExistingContainers(): Promise<void> {
Expand Down