code review fixes

Author: gallayl

frontend/src/utils/prerequisite-summary.spec.ts

@@ -0,0 +1,68 @@
+import { describe, expect, it } from 'vitest'
+
+import type { PrerequisiteCheckResult } from 'common'
+
+import { getPrerequisiteSummary } from './prerequisite-summary.js'
+
+const makeCheckResult = (
+  prerequisiteId: string,
+  status: PrerequisiteCheckResult['status'],
+): PrerequisiteCheckResult => ({
+  prerequisiteId,
+  status,
+  output: '',
+  checkedAt: '',
+})
+
+describe('getPrerequisiteSummary', () => {
+  it('should return null for empty prereqIds', () => {
+    const result = getPrerequisiteSummary([], new Map())
+    expect(result).toBeNull()
+  })
+
+  it('should return all satisfied when every prereq is satisfied', () => {
+    const map = new Map<string, PrerequisiteCheckResult>([
+      ['p1', makeCheckResult('p1', 'satisfied')],
+      ['p2', makeCheckResult('p2', 'satisfied')],
+    ])
+    const result = getPrerequisiteSummary(['p1', 'p2'], map)
+    expect(result).toEqual({ satisfiedCount: 2, failedCount: 0, total: 2 })
+  })
+
+  it('should count failed prerequisites', () => {
+    const map = new Map<string, PrerequisiteCheckResult>([
+      ['p1', makeCheckResult('p1', 'satisfied')],
+      ['p2', makeCheckResult('p2', 'failed')],
+      ['p3', makeCheckResult('p3', 'failed')],
+    ])
+    const result = getPrerequisiteSummary(['p1', 'p2', 'p3'], map)
+    expect(result).toEqual({ satisfiedCount: 1, failedCount: 2, total: 3 })
+  })
+
+  it('should treat missing map entries as unchecked (neither satisfied nor failed)', () => {
+    const map = new Map<string, PrerequisiteCheckResult>([['p1', makeCheckResult('p1', 'satisfied')]])
+    const result = getPrerequisiteSummary(['p1', 'p2'], map)
+    expect(result).toEqual({ satisfiedCount: 1, failedCount: 0, total: 2 })
+  })
+
+  it('should treat "checking" and "unchecked" statuses as neither satisfied nor failed', () => {
+    const map = new Map<string, PrerequisiteCheckResult>([
+      ['p1', makeCheckResult('p1', 'checking')],
+      ['p2', makeCheckResult('p2', 'unchecked')],
+      ['p3', makeCheckResult('p3', 'satisfied')],
+    ])
+    const result = getPrerequisiteSummary(['p1', 'p2', 'p3'], map)
+    expect(result).toEqual({ satisfiedCount: 1, failedCount: 0, total: 3 })
+  })
+
+  it('should handle a mix of all statuses', () => {
+    const map = new Map<string, PrerequisiteCheckResult>([
+      ['p1', makeCheckResult('p1', 'satisfied')],
+      ['p2', makeCheckResult('p2', 'failed')],
+      ['p3', makeCheckResult('p3', 'checking')],
+      ['p4', makeCheckResult('p4', 'unchecked')],
+    ])
+    const result = getPrerequisiteSummary(['p1', 'p2', 'p3', 'p4', 'p5'], map)
+    expect(result).toEqual({ satisfiedCount: 1, failedCount: 1, total: 5 })
+  })
+})

service/src/services/process-manager.ts

@@ -472,8 +472,6 @@ export class ProcessManager {
   }
 
   public async [Symbol.asyncDispose]() {
-    await this.runner[Symbol.asyncDispose]()
-
     await this.logger.information({ message: 'Disposing ProcessManager, killing all child processes...' })
 
     const entries = [...this.runner.processes.entries()]
@@ -523,6 +521,8 @@ export class ProcessManager {
       this.runner.processes.clear()
     }
 
+    await this.runner[Symbol.asyncDispose]()
+
     try {
       await this.elevatedInjector?.[Symbol.asyncDispose]()
     } catch {

service/src/utils/secret-detector.ts

@@ -1,9 +1,6 @@
-export type SecretWarning = {
-  line: number
-  pattern: string
-  snippet: string
-  suggestion?: string
-}
+import type { SecretWarning } from 'common'
+
+type SecretPatternMatch = Omit<SecretWarning, 'source'>
 
 const SECRET_PATTERNS: Array<{ regex: RegExp; label: string }> = [
   { regex: /(?:password|passwd|pwd)\s*[=:]\s*\S+/i, label: 'password assignment' },
@@ -23,9 +20,9 @@ const MAX_SNIPPET_LENGTH = 80
  * Returns warnings with line numbers, the matched pattern name, and a
  * truncated snippet of the matching line.
  */
-export const detectSecretPatterns = (text: string): SecretWarning[] => {
+export const detectSecretPatterns = (text: string): SecretPatternMatch[] => {
   const lines = text.split('\n')
-  const warnings: SecretWarning[] = []
+  const warnings: SecretPatternMatch[] = []
 
   for (let i = 0; i < lines.length; i++) {
     const line = lines[i]
@@ -52,8 +49,8 @@ export const detectSecretsInServiceDefinition = (opts: {
   runCommand?: string
   installCommand?: string
   buildCommand?: string
-}): Array<SecretWarning & { source: string }> => {
-  const results: Array<SecretWarning & { source: string }> = []
+}): SecretWarning[] => {
+  const results: SecretWarning[] = []
 
   for (const file of opts.files ?? []) {
     for (const warning of detectSecretPatterns(file.content)) {