pr feedback

Author: frostebite

dist/index.js

@@ -209,6 +209,7 @@ class AWSTaskRunner {
         const sleepMs = baseBackoffMs + jitterMs;
         CloudRunnerLogger.log(`AWS throttled GetRecords, backing off ${sleepMs}ms (1000 + jitter ${jitterMs})`);
         await new Promise((r) => setTimeout(r, sleepMs));
+
         return { iterator, shouldReadLogs, output, shouldCleanup };
       }
       throw error;

dist/index.js.map

@@ -204,6 +204,7 @@ export class TaskService {
       const objects = await (
         SharedWorkspaceLocking as unknown as { listObjects(prefix: string): Promise<string[]> }
       ).listObjects('');
+
       return objects.map((x: string) => ({ Key: x }));
     }
     const s3 = AwsClientFactory.getS3();
@@ -213,6 +214,6 @@ export class TaskService {
 
     const results = await s3.send(new ListObjectsV2Command(listRequest));
 
-    return (results.Contents || []).map((obj) => ({ Key: obj.Key || '' }));
+    return (results.Contents || []).map((object) => ({ Key: object.Key || '' }));
   }
 }

src/model/cloud-runner/providers/aws/aws-task-runner.ts

@@ -22,7 +22,7 @@ class KubernetesJobSpecFactory {
     containerName: string,
     ip: string = '',
   ) {
-    const endpointEnvNames = new Set([
+    const endpointEnvironmentNames = new Set([
       'AWS_S3_ENDPOINT',
       'AWS_ENDPOINT',
       'AWS_CLOUD_FORMATION_ENDPOINT',
@@ -36,7 +36,7 @@ class KubernetesJobSpecFactory {
       let value = x.value;
       if (
         typeof value === 'string' &&
-        endpointEnvNames.has(x.name) &&
+        endpointEnvironmentNames.has(x.name) &&
         (value.startsWith('http://localhost') || value.startsWith('http://127.0.0.1'))
       ) {
         // Replace localhost with host.k3d.internal so pods can access host services
@@ -45,6 +45,7 @@ class KubernetesJobSpecFactory {
           .replace('http://localhost', 'http://host.k3d.internal')
           .replace('http://127.0.0.1', 'http://host.k3d.internal');
       }
+
       return { name: x.name, value } as CloudRunnerEnvironmentVariable;
     });
 

src/model/cloud-runner/providers/aws/services/task-service.ts

@@ -19,8 +19,7 @@ class KubernetesPods {
         }));
 
       const errorDetails: string[] = [];
-      errorDetails.push(`Pod: ${podName}`);
-      errorDetails.push(`Phase: ${phase}`);
+      errorDetails.push(`Pod: ${podName}`, `Phase: ${phase}`);
 
       if (conditions.length > 0) {
         errorDetails.push(
@@ -36,10 +35,10 @@ class KubernetesPods {
       let containerSucceeded = false;
 
       if (containerStatuses.length > 0) {
-        containerStatuses.forEach((cs, idx) => {
+        for (const [index, cs] of containerStatuses.entries()) {
           if (cs.state?.waiting) {
             errorDetails.push(
-              `Container ${idx} (${cs.name}) waiting: ${cs.state.waiting.reason} - ${cs.state.waiting.message || ''}`,
+              `Container ${index} (${cs.name}) waiting: ${cs.state.waiting.reason} - ${cs.state.waiting.message || ''}`,
             );
           }
           if (cs.state?.terminated) {
@@ -49,12 +48,12 @@ class KubernetesPods {
               containerSucceeded = true;
             }
             errorDetails.push(
-              `Container ${idx} (${cs.name}) terminated: ${cs.state.terminated.reason} - ${
+              `Container ${index} (${cs.name}) terminated: ${cs.state.terminated.reason} - ${
                 cs.state.terminated.message || ''
               } (exit code: ${exitCode})`,
             );
           }
-        });
+        }
       }
 
       if (events.length > 0) {
@@ -80,6 +79,7 @@ class KubernetesPods {
           );
         }
         CloudRunnerLogger.log(`Pod details: ${errorDetails.join('\n')}`);
+
         // Don't throw error - container succeeded, PreStopHook failure is non-critical
         return false; // Pod is not running, but we don't treat it as a failure
       }
@@ -90,8 +90,9 @@ class KubernetesPods {
         CloudRunnerLogger.log(
           `Pod ${podName} was killed with PreStopHook failure. Waiting for container status to determine if container succeeded...`,
         );
+
         // Wait a bit for container status to become available (up to 30 seconds)
-        for (let i = 0; i < 6; i++) {
+        for (let index = 0; index < 6; index++) {
           await new Promise((resolve) => setTimeout(resolve, 5000));
           try {
             const updatedPod = (await kubeClient.listNamespacedPod(namespace)).body.items.find(
@@ -105,6 +106,7 @@ class KubernetesPods {
                   CloudRunnerLogger.logWarning(
                     `Pod ${podName} container succeeded (exit code 0) after waiting. PreStopHook failure is non-fatal.`,
                   );
+
                   return false; // Pod is not running, but container succeeded
                 } else {
                   CloudRunnerLogger.log(
@@ -121,12 +123,14 @@ class KubernetesPods {
             CloudRunnerLogger.log(`Error while waiting for container status: ${waitError}`);
           }
         }
+
         // If we still don't have container status after waiting, but only PreStopHook failed,
         // be lenient - the container might have succeeded but status wasn't updated
         if (containerExitCode === undefined && hasPreStopHookFailure && !hasExceededGracePeriod) {
           CloudRunnerLogger.logWarning(
             `Pod ${podName} container status not available after waiting, but only PreStopHook failed (no ExceededGracePeriod). Assuming container may have succeeded.`,
           );
+
           return false; // Be lenient - PreStopHook failure alone is not fatal
         }
         CloudRunnerLogger.log(
@@ -139,6 +143,7 @@ class KubernetesPods {
         CloudRunnerLogger.logWarning(
           `Pod ${podName} has PreStopHook failure but no container failure detected. Treating as non-fatal.`,
         );
+
         return false; // PreStopHook failure alone is not fatal if container status is unclear
       }
 
@@ -149,8 +154,10 @@ class KubernetesPods {
         CloudRunnerLogger.logWarning(
           `Pod ${podName} was killed (exit code 137 - likely OOM or resource limit) with PreStopHook/grace period issues. This may be a resource constraint issue rather than a build failure.`,
         );
+
         // Still log the details but don't fail the test - the build might have succeeded before being killed
         CloudRunnerLogger.log(`Pod details: ${errorDetails.join('\n')}`);
+
         return false; // Don't treat system kills as test failures if only PreStopHook issues
       }
 

src/model/cloud-runner/providers/k8s/kubernetes-job-spec-factory.ts

@@ -62,25 +62,30 @@ class KubernetesTaskRunner {
               true,
               callback,
             );
+
             // If we successfully got logs, check for end of transmission
             if (FollowLogStreamService.DidReceiveEndOfTransmission) {
               CloudRunnerLogger.log('end of log stream');
               break;
             }
+
             // If we got logs but no end marker, continue trying (might be more logs)
             if (retriesAfterFinish < KubernetesTaskRunner.maxRetry) {
               retriesAfterFinish++;
               continue;
             }
+
             // If we've exhausted retries, break
             break;
           } catch (fallbackError: any) {
             CloudRunnerLogger.log(`Fallback log fetch also failed: ${fallbackError}`);
+
             // If both fail, continue retrying if we haven't exhausted retries
             if (retriesAfterFinish < KubernetesTaskRunner.maxRetry) {
               retriesAfterFinish++;
               continue;
             }
+
             // Only break if we've exhausted all retries
             CloudRunnerLogger.logWarning(
               `Could not fetch any container logs after ${KubernetesTaskRunner.maxRetry} retries`,
@@ -101,6 +106,7 @@ class KubernetesTaskRunner {
         if (!error?.message?.includes('previous terminated container')) {
           throw error;
         }
+
         // For previous container errors, we've already tried fallback, so just break
         CloudRunnerLogger.logWarning(
           `Could not fetch previous container logs after retries, but continuing with available logs`,

src/model/cloud-runner/providers/k8s/kubernetes-pods.ts

@@ -74,8 +74,10 @@ class LocalCloudRunner implements ProviderInterface {
         .split('\n')
         .filter((x) => x.trim().length > 0)
         .join(' ; ');
+
       // Use shell-quote to properly escape the command string, preventing command injection
       const bashWrapped = `bash -lc ${quote([inline])}`;
+
       return await CloudRunnerSystem.Run(bashWrapped);
     }
 

src/model/cloud-runner/providers/k8s/kubernetes-task-runner.ts

@@ -84,12 +84,13 @@ export class Caching {
       try {
         const diskCheckOutput = await CloudRunnerSystem.Run(`df . 2>/dev/null || df /data 2>/dev/null || true`);
         CloudRunnerLogger.log(`Disk space before tar: ${diskCheckOutput}`);
+
         // Parse disk usage percentage (e.g., "72G  72G  196M 100%")
         const usageMatch = diskCheckOutput.match(/(\d+)%/);
         if (usageMatch) {
-          diskUsagePercent = parseInt(usageMatch[1], 10);
+          diskUsagePercent = Number.parseInt(usageMatch[1], 10);
         }
-      } catch (error) {
+      } catch {
         // Ignore disk check errors
       }
 
@@ -103,6 +104,7 @@ export class Caching {
             await CloudRunnerSystem.Run(
               `find ${cacheParent} -name "*.tar*" -type f -mmin +360 -delete 2>/dev/null || true`,
             );
+
             // Also try to remove old cache directories
             await CloudRunnerSystem.Run(`find ${cacheParent} -type d -empty -delete 2>/dev/null || true`);
             CloudRunnerLogger.log(`Cleanup completed. Checking disk space again...`);
@@ -117,7 +119,7 @@ export class Caching {
       // Clean up any existing incomplete tar files
       try {
         await CloudRunnerSystem.Run(`rm -f ${cacheArtifactName}.tar${compressionSuffix} 2>/dev/null || true`);
-      } catch (error) {
+      } catch {
         // Ignore cleanup errors
       }
 
@@ -130,6 +132,7 @@ export class Caching {
         const errorMessage = error?.message || error?.toString() || '';
         if (errorMessage.includes('No space left') || errorMessage.includes('Wrote only')) {
           CloudRunnerLogger.log(`Disk space error detected. Attempting aggressive cleanup...`);
+
           // Try to clean up old cache files more aggressively
           try {
             const cacheParent = path.dirname(cacheFolder);
@@ -138,8 +141,10 @@ export class Caching {
               await CloudRunnerSystem.Run(
                 `find ${cacheParent} -name "*.tar*" -type f -mmin +60 -delete 2>/dev/null || true`,
               );
+
               // Remove empty cache directories
               await CloudRunnerSystem.Run(`find ${cacheParent} -type d -empty -delete 2>/dev/null || true`);
+
               // Also try to clean up the entire cache folder if it's getting too large
               const cacheRoot = path.resolve(cacheParent, '..');
               if (await fileExists(cacheRoot)) {
@@ -149,12 +154,14 @@ export class Caching {
                 );
               }
               CloudRunnerLogger.log(`Aggressive cleanup completed. Retrying tar operation...`);
+
               // Retry the tar operation once after cleanup
               let retrySucceeded = false;
               try {
                 await CloudRunnerSystem.Run(
                   `tar -cf ${cacheArtifactName}.tar${compressionSuffix} "${path.basename(sourceFolder)}"`,
                 );
+
                 // If retry succeeds, mark it - we'll continue normally without throwing
                 retrySucceeded = true;
               } catch (retryError: any) {
@@ -164,10 +171,12 @@ export class Caching {
                   }`,
                 );
               }
+
               // If retry succeeded, don't throw the original error - let execution continue after catch block
               if (!retrySucceeded) {
                 throw error;
               }
+
               // If we get here, retry succeeded - execution will continue after the catch block
             } else {
               throw new Error(
@@ -189,6 +198,7 @@ export class Caching {
       await CloudRunnerSystem.Run(`du ${cacheArtifactName}.tar${compressionSuffix}`);
       assert(await fileExists(`${cacheArtifactName}.tar${compressionSuffix}`), 'cache archive exists');
       assert(await fileExists(path.basename(sourceFolder)), 'source folder exists');
+
       // Ensure the cache folder directory exists before moving the file
       // (it might have been deleted by cleanup if it was empty)
       if (!(await fileExists(cacheFolder))) {