Skip to content

fix: handle missing docker-compose.yml during cleanup gracefully #741

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

Merged
lpcox merged 2 commits into from
Feb 12, 2026
+30 −5
Merged

fix: handle missing docker-compose.yml during cleanup gracefully #741

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
077d032
Initial plan
Claude Feb 12, 2026
dde29f5
fix: handle missing docker-compose.yml during cleanup gracefully
Claude Feb 12, 2026
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
35 changes: 30 additions & 5 deletions src/docker-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,11 +1301,36 @@ export async function stopContainers(workDir: string, keepContainers: boolean):
logger.info('Stopping containers...');

try {
await execa('docker', ['compose', 'down', '-v'], {
cwd: workDir,
stdio: 'inherit',
});
logger.success('Containers stopped successfully');
// Check if workDir and docker-compose.yml exist before using docker compose
const composeFile = path.join(workDir, 'docker-compose.yml');
if (fs.existsSync(workDir) && fs.existsSync(composeFile)) {
// Normal path: use docker compose down
await execa('docker', ['compose', 'down', '-v'], {
cwd: workDir,
stdio: 'inherit',
});
logger.success('Containers stopped successfully');
} else {
// Fallback: compose file missing, stop containers by name
logger.debug('Compose file not found, stopping containers by name');

// Stop and remove containers by name
const containerNames = ['awf-agent', 'awf-squid'];
for (const name of containerNames) {
try {
// Check if container exists
const { stdout } = await execa('docker', ['ps', '-aq', '-f', `name=^${name}$`]);
if (stdout.trim()) {
logger.debug(`Stopping container: ${name}`);
await execa('docker', ['rm', '-f', name], { stdio: 'inherit' });
}
} catch (err) {
logger.debug(`Could not stop container ${name}:`, err);
}
}

logger.success('Containers stopped successfully');
Comment on lines +1317 to +1332
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

The error handling in the fallback path swallows all errors with individual try-catch blocks (lines 1320-1329). This means if both containers fail to stop (e.g., Docker daemon is down), the function will still log "Containers stopped successfully" and return without error.

Consider tracking whether at least one container operation succeeded, or accumulating errors and throwing if no containers could be stopped. For example:

let stoppedAny = false;
const errors: Error[] = [];
for (const name of containerNames) {
  try {
    // ... existing logic ...
    stoppedAny = true;
  } catch (err) {
    errors.push(err);
    logger.debug(`Could not stop container ${name}:`, err);
  }
}
if (!stoppedAny && errors.length > 0) {
  throw new Error(`Failed to stop any containers: ${errors.map(e => e.message).join(', ')}`);
}

This ensures the outer error handler and calling code are aware when cleanup truly fails.

Copilot uses AI. Check for mistakes.
}
Comment on lines +1304 to +1333
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

The logic assumes the fallback path (stopping containers by name) is for error scenarios, but due to the security fix at line 1149 that deletes docker-compose.yml immediately after containers start, the compose file will NOT exist during normal cleanup. This makes the fallback path the primary execution path, not a true fallback.

Consider either:

  1. Removing the compose file check and always using docker rm to stop containers by name (simpler, more predictable)
  2. Keeping the compose file but restricting its permissions instead of deleting it
  3. Adding a comment explaining that this is the expected normal path, not just a fallback

The current implementation works correctly but the code structure and comments are misleading about which path is the normal case versus the exceptional case.

This issue also appears on line 1307 of the same file.

Copilot uses AI. Check for mistakes.
} catch (error) {
logger.error('Failed to stop containers:', error);
throw error;
Expand Down
Loading