refactor(common): consolidate browser-actions parsing (Commit 2.13)

- Create parseActionValue() utility for string→type conversion
- Create LAZY_EDIT_PATTERNS constant for pattern matching
- Update parseBrowserActionXML and parseBrowserActionAttributes
- Simplify hasLazyEdit() and replaceNonStandardPlaceholderComments()
- Add comprehensive unit tests

Author: brandonkachen

common/src/__tests__/browser-actions.test.ts

@@ -0,0 +1,142 @@
+import { describe, expect, it } from 'bun:test'
+
+import { parseActionValue } from '../browser-actions'
+
+describe('parseActionValue', () => {
+  it('should parse boolean strings', () => {
+    expect(parseActionValue('true')).toBe(true)
+    expect(parseActionValue('false')).toBe(false)
+  })
+
+  it('should parse numeric strings', () => {
+    expect(parseActionValue('42')).toBe(42)
+    expect(parseActionValue('3.14')).toBe(3.14)
+    expect(parseActionValue('0')).toBe(0)
+    expect(parseActionValue('-10')).toBe(-10)
+    expect(parseActionValue('-3.14')).toBe(-3.14)
+  })
+
+  it('should parse JSON objects', () => {
+    expect(parseActionValue('{"key":"value"}')).toEqual({ key: 'value' })
+    expect(parseActionValue('{"timeout":5000}')).toEqual({ timeout: 5000 })
+  })
+
+  it('should parse JSON arrays', () => {
+    expect(parseActionValue('[1,2,3]')).toEqual([1, 2, 3])
+    expect(parseActionValue('["a","b"]')).toEqual(['a', 'b'])
+  })
+
+  it('should return invalid JSON as string', () => {
+    expect(parseActionValue('{invalid}')).toBe('{invalid}')
+    expect(parseActionValue('[incomplete')).toBe('[incomplete')
+  })
+
+  it('should keep regular strings as strings', () => {
+    expect(parseActionValue('hello')).toBe('hello')
+    expect(parseActionValue('https://example.com')).toBe('https://example.com')
+    expect(parseActionValue('')).toBe('')
+  })
+
+  it('should not parse strings that look like numbers but are not', () => {
+    // Mixed alphanumeric strings should remain as strings
+    expect(parseActionValue('123abc')).toBe('123abc')
+  })
+
+  it('should keep empty string as empty string', () => {
+    expect(parseActionValue('')).toBe('')
+  })
+
+  // Edge case tests for strict numeric parsing
+  describe('numeric parsing edge cases', () => {
+    it('should keep whitespace-only strings as strings', () => {
+      expect(parseActionValue('   ')).toBe('   ')
+      expect(parseActionValue(' ')).toBe(' ')
+      expect(parseActionValue('\t')).toBe('\t')
+      expect(parseActionValue('\n')).toBe('\n')
+    })
+
+    it('should keep hex strings as strings', () => {
+      expect(parseActionValue('0x10')).toBe('0x10')
+      expect(parseActionValue('0xFF')).toBe('0xFF')
+      expect(parseActionValue('0xABC')).toBe('0xABC')
+    })
+
+    it('should keep binary strings as strings', () => {
+      expect(parseActionValue('0b10')).toBe('0b10')
+      expect(parseActionValue('0b1111')).toBe('0b1111')
+    })
+
+    it('should keep octal strings as strings', () => {
+      expect(parseActionValue('0o10')).toBe('0o10')
+      expect(parseActionValue('0o777')).toBe('0o777')
+    })
+
+    it('should keep Infinity as string', () => {
+      expect(parseActionValue('Infinity')).toBe('Infinity')
+      expect(parseActionValue('-Infinity')).toBe('-Infinity')
+    })
+
+    it('should keep NaN as string', () => {
+      expect(parseActionValue('NaN')).toBe('NaN')
+    })
+
+    it('should keep scientific notation as string', () => {
+      expect(parseActionValue('1e10')).toBe('1e10')
+      expect(parseActionValue('1E10')).toBe('1E10')
+      expect(parseActionValue('1e+10')).toBe('1e+10')
+      expect(parseActionValue('1e-10')).toBe('1e-10')
+      expect(parseActionValue('2.5e3')).toBe('2.5e3')
+    })
+
+    it('should keep explicit positive sign as string', () => {
+      expect(parseActionValue('+5')).toBe('+5')
+      expect(parseActionValue('+3.14')).toBe('+3.14')
+    })
+
+    it('should parse numbers with leading zeros as numbers', () => {
+      // Leading zeros are allowed and parsed as decimal numbers
+      expect(parseActionValue('007')).toBe(7)
+      expect(parseActionValue('00')).toBe(0)
+      expect(parseActionValue('0123')).toBe(123)
+    })
+
+    it('should keep strings with embedded whitespace as strings', () => {
+      expect(parseActionValue('  42  ')).toBe('  42  ')
+      expect(parseActionValue('4 2')).toBe('4 2')
+    })
+  })
+
+  // Edge case tests for JSON parsing
+  describe('JSON parsing edge cases', () => {
+    it('should parse nested JSON objects', () => {
+      expect(parseActionValue('{"a":{"b":{"c":1}}}')).toEqual({
+        a: { b: { c: 1 } },
+      })
+    })
+
+    it('should parse nested JSON arrays', () => {
+      expect(parseActionValue('[[1,2],[3,4]]')).toEqual([
+        [1, 2],
+        [3, 4],
+      ])
+    })
+
+    it('should parse JSON with mixed types', () => {
+      expect(parseActionValue('[1,"a",true,null]')).toEqual([1, 'a', true, null])
+    })
+
+    it('should parse JSON with special characters', () => {
+      expect(parseActionValue('{"key":"value with spaces"}')).toEqual({
+        key: 'value with spaces',
+      })
+      expect(parseActionValue('{"key":"line1\\nline2"}')).toEqual({
+        key: 'line1\nline2',
+      })
+    })
+
+    it('should parse empty JSON structures', () => {
+      expect(parseActionValue('{}')).toEqual({})
+      expect(parseActionValue('[]')).toEqual([])
+    })
+  })
+})

common/src/browser-actions.ts

@@ -359,30 +359,10 @@ export function parseBrowserActionXML(xmlString: string): BrowserAction {
   // Parse special values (booleans, numbers, objects)
   const parsedAttrs = Object.entries(attrs).reduce(
     (acc, [key, value]) => {
-      try {
-        // Try to parse as JSON for objects
-        if (value.startsWith('{') || value.startsWith('[')) {
-          acc[key] = JSON.parse(value)
-        }
-        // Parse booleans
-        else if (value === 'true' || value === 'false') {
-          acc[key] = value === 'true'
-        }
-        // Parse numbers
-        else if (!isNaN(Number(value))) {
-          acc[key] = Number(value)
-        }
-        // Keep as string
-        else {
-          acc[key] = value
-        }
-      } catch {
-        // If parsing fails, keep as string
-        acc[key] = value
-      }
+      acc[key] = parseActionValue(value)
       return acc
     },
-    {} as Record<string, any>,
+    {} as Record<string, unknown>,
   )
 
   // Construct and validate the BrowserAction
@@ -393,6 +373,38 @@ export function parseBrowserActionXML(xmlString: string): BrowserAction {
 export type BrowserResponse = z.infer<typeof BrowserResponseSchema>
 export type BrowserAction = z.infer<typeof BrowserActionSchema>
 
+/**
+ * Strict regex for decimal numbers: optional minus, one or more digits, optional decimal part.
+ * This prevents parsing of hex (0x10), binary (0b10), octal (0o10), Infinity, NaN,
+ * scientific notation (1e10), and whitespace strings.
+ */
+const STRICT_DECIMAL_REGEX = /^-?\d+(\.\d+)?$/
+
+/**
+ * Parse a string value into its appropriate JS type (boolean, number, object, or string)
+ */
+export function parseActionValue(value: string): unknown {
+  // Try to parse as JSON for objects/arrays
+  if (value.startsWith('{') || value.startsWith('[')) {
+    try {
+      return JSON.parse(value)
+    } catch {
+      return value
+    }
+  }
+  // Parse booleans
+  if (value === 'true') return true
+  if (value === 'false') return false
+  // Parse numbers using strict decimal regex to avoid edge cases:
+  // - Whitespace strings: Number('   ') === 0
+  // - Hex strings: Number('0x10') === 16
+  // - Infinity: Number('Infinity') === Infinity
+  // - Scientific notation: Number('1e10') === 10000000000
+  if (STRICT_DECIMAL_REGEX.test(value)) return Number(value)
+  // Keep as string
+  return value
+}
+
 /**
  * Parse browser action XML attributes into a typed BrowserAction object
  */
@@ -402,12 +414,12 @@ export function parseBrowserActionAttributes(
   const { action, ...rest } = attributes
   return {
     type: action,
-    ...Object.entries(rest).reduce((acc, [key, value]) => {
-      // Convert string values to appropriate types
-      if (value === 'true') return { ...acc, [key]: true }
-      if (value === 'false') return { ...acc, [key]: false }
-      if (!isNaN(Number(value))) return { ...acc, [key]: Number(value) }
-      return { ...acc, [key]: value }
-    }, {}),
+    ...Object.entries(rest).reduce(
+      (acc, [key, value]) => {
+        acc[key] = parseActionValue(value)
+        return acc
+      },
+      {} as Record<string, unknown>,
+    ),
   } as BrowserAction
 }

common/src/util/__tests__/string.test.ts

@@ -1,7 +1,12 @@
 import { describe, expect, it } from 'bun:test'
 
 import { EXISTING_CODE_MARKER } from '../../old-constants'
-import { pluralize, replaceNonStandardPlaceholderComments } from '../string'
+import {
+  hasLazyEdit,
+  LAZY_EDIT_PATTERNS,
+  pluralize,
+  replaceNonStandardPlaceholderComments,
+} from '../string'
 
 describe('pluralize', () => {
   it('should handle singular and plural cases correctly', () => {
@@ -238,6 +243,62 @@ describe('pluralize', () => {
   })
 })
 
+describe('LAZY_EDIT_PATTERNS', () => {
+  it('should detect C-style single-line comments', () => {
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('// ... existing code ...'))).toBe(true)
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('// ... rest of file'))).toBe(true)
+  })
+
+  it('should detect C-style multi-line comments', () => {
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('/* ... unchanged ... */'))).toBe(true)
+  })
+
+  it('should detect Python/Ruby comments', () => {
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('# ... existing code ...'))).toBe(true)
+  })
+
+  it('should detect HTML comments', () => {
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('<!-- ... keep ... -->'))).toBe(true)
+  })
+
+  it('should detect JSX comments', () => {
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('{/* ... some code ... */'))).toBe(true)
+  })
+
+  it('should detect SQL/Haskell comments', () => {
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('-- ... file ...'))).toBe(true)
+  })
+
+  it('should detect MATLAB comments', () => {
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('% ... rest ...'))).toBe(true)
+  })
+
+  it('should not match regular comments', () => {
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('// regular comment'))).toBe(false)
+    expect(LAZY_EDIT_PATTERNS.some((p) => p.test('/* normal comment */'))).toBe(false)
+  })
+})
+
+describe('hasLazyEdit', () => {
+  it('should detect common lazy edit patterns', () => {
+    expect(hasLazyEdit('... existing code ...')).toBe(true)
+    expect(hasLazyEdit('// rest of the file')).toBe(true)
+    expect(hasLazyEdit('# rest of the function')).toBe(true)
+  })
+
+  it('should detect regex-based lazy edit patterns', () => {
+    expect(hasLazyEdit('// ... existing code ...')).toBe(true)
+    expect(hasLazyEdit('/* ... unchanged ... */')).toBe(true)
+    expect(hasLazyEdit('# ... keep ...')).toBe(true)
+    expect(hasLazyEdit('<!-- ... file ... -->')).toBe(true)
+  })
+
+  it('should not detect regular code', () => {
+    expect(hasLazyEdit('const x = 1')).toBe(false)
+    expect(hasLazyEdit('// this is a comment')).toBe(false)
+  })
+})
+
 describe('replaceNonStandardPlaceholderComments', () => {
   it('should replace C-style comments', () => {
     const input = `