refactor: refine path validation heuristics to reduce false positives

packages/cli/src/utils/atCommandUtils.test.ts

@@ -194,4 +194,31 @@ describe('atCommandUtils', () => {
       );
     }
   });
+
+  it('should not treat paths with shared prefixes as subpaths if not actually inside', async () => {
+    // /mock/root-backup/file.txt starts with /mock/root but is not inside it.
+    const dir = '/mock/root';
+    const otherPath = '/mock/root-backup/file.txt';
+
+    (mockWorkspaceContext['getDirectories'] as Mock).mockReturnValue([dir]);
+    const mockStats = {
+      isDirectory: () => false,
+      isFile: () => true,
+    };
+    vi.mocked(fsPromises.stat).mockResolvedValue(mockStats as unknown as Stats);
+
+    const result = await resolveAtCommandPath(
+      otherPath,
+      mockConfig as unknown as Config,
+    );
+
+    expect(result.status).toBe('resolved');
+    if (result.status === 'resolved') {
+      expect(result.resolved.absolutePath).toBe(otherPath);
+      // It should NOT be relative to /mock/root because it's not actually inside it.
+      // path.relative('/mock/root', '/mock/root-backup/file.txt') -> '../root-backup/file.txt'
+      // Our fix should prevent this from being used as a relative path.
+      expect(result.resolved.relativePath).toBe(otherPath);
+    }
+  });
 });

packages/cli/src/utils/atCommandUtils.ts

@@ -63,8 +63,9 @@ export async function resolveAtCommandPath(
       // Try to find if it's within one of the workspace directories to provide a nice relative path
       let relativePath = pathName;
       for (const dir of workspaceDirs) {
-        if (pathName.startsWith(dir)) {
-          relativePath = path.relative(dir, pathName);
+        const rel = path.relative(dir, pathName);
+        if (!rel.startsWith('..') && !path.isAbsolute(rel)) {
+          relativePath = rel;
           break;
         }
       }

packages/core/src/utils/path-validator.test.ts

@@ -43,7 +43,38 @@ describe('PathValidator', () => {
     );
   });
 
-  it('should reject misinterpreted log fragments with quotes or ellipses', () => {
+  it('should allow paths with single quotes (apostrophes)', () => {
+    // This was previously a false positive
+    expect(validatePath("/Users/john's_files/project/index.ts").isValid).toBe(
+      true,
+    );
+  });
+
+  it('should allow long paths with brackets or parentheses', () => {
+    // These were previously false positives (Next.js dynamic routes, Windows copies)
+    expect(
+      validatePath('packages/web/app/dashboard/[id]/settings/page.tsx').isValid,
+    ).toBe(true);
+    expect(
+      validatePath('/Users/name/Documents/Project (Copy)/index.ts').isValid,
+    ).toBe(true);
+  });
+
+  it('should only reject log markers at the start of the path', () => {
+    // Legitimate paths containing these strings should now be allowed
+    expect(validatePath('src/tests/FAIL_CASE.txt').isValid).toBe(true);
+    expect(validatePath('FAILURE_LOG.txt').isValid).toBe(true);
+    expect(validatePath('docs/AssertionError_details.md').isValid).toBe(true);
+
+    // But they should still be rejected at the start
+    expect(validatePath('FAIL tests/int/my.test.ts').isValid).toBe(false);
+    expect(
+      validatePath('AssertionError: expected true to be false').isValid,
+    ).toBe(false);
+    expect(validatePath('✓ test passed').isValid).toBe(false);
+  });
+
+  it('should reject misinterpreted log fragments with double quotes or ellipses', () => {
     const logFragment =
       'Error: No "formatTimeRange" export is defined on the lib/formatTimeRange mock.';
     const result = validatePath(logFragment);
@@ -51,30 +82,7 @@ describe('PathValidator', () => {
     expect(result.error).toContain('suspicious characters');
   });
 
-  it('should reject code snippets with braces/parens', () => {
-    const codeSnippet =
-      'vi.mock(import("@/lib/formatTimeRange"), async (importOriginal) => { return { ...actual }; })';
-    const result = validatePath(codeSnippet);
-    expect(result.isValid).toBe(false);
-    expect(result.error).toContain(
-      'misinterpreted log fragment or code snippet',
-    );
-  });
-
-  it('should reject misinterpreted log fragments with log markers', () => {
-    expect(validatePath('FAIL tests/int/my.test.ts').isValid).toBe(false);
-    expect(
-      validatePath('AssertionError: expected true to be false').isValid,
-    ).toBe(false);
-    expect(validatePath('✓ test passed').isValid).toBe(false);
-    expect(validatePath('× test failed').isValid).toBe(false);
-    expect(
-      validatePath('TestingLibraryElementError: Unable to find an element')
-        .isValid,
-    ).toBe(false);
-  });
-
-  it('should allow short paths with quotes (even if unusual)', () => {
+  it('should allow short paths with double quotes (even if unusual)', () => {
     // Some systems might technically allow this, and we only want to block long/obvious log fragments
     expect(validatePath('file"with"quote.txt').isValid).toBe(true);
   });
@@ -85,8 +93,4 @@ describe('PathValidator', () => {
         .isValid,
     ).toBe(false);
   });
-
-  it('should allow short paths with braces', () => {
-    expect(validatePath('{a,b}.ts').isValid).toBe(true);
-  });
 });

packages/core/src/utils/path-validator.ts

@@ -40,15 +40,17 @@ export function validatePath(pathStr: string): PathValidationResult {
   }
 
   // Check for common log/error patterns that are definitely not paths
-  const logMarkers = [
-    'AssertionError',
-    'FAIL',
-    '✓',
-    '×',
-    'TestingLibraryElementError',
+  // We use anchored regex to ensure we only block if these appear at the start,
+  // which is typical for misinterpreted log lines.
+  const logMarkerRegexes = [
+    /^AssertionError:/,
+    /^FAIL /,
+    /^✓ /,
+    /^× /,
+    /^TestingLibraryElementError:/,
   ];
-  for (const marker of logMarkers) {
-    if (pathStr.includes(marker)) {
+  for (const regex of logMarkerRegexes) {
+    if (regex.test(pathStr)) {
       return {
         isValid: false,
         error: 'Path appears to be a misinterpreted log fragment.',
@@ -56,26 +58,14 @@ export function validatePath(pathStr: string): PathValidationResult {
     }
   }
 
-  // If it contains characters like '{', '}', '(', ')', '[', ']' and it's long, it's likely a log
-  if (pathStr.length > 50 && /[{}[\]()]/.test(pathStr)) {
-    return {
-      isValid: false,
-      error:
-        'Path appears to be a misinterpreted log fragment or code snippet.',
-    };
-  }
-
-  // Check for quotes or ellipses in "paths" - almost always a misinterpretation if not a very short name
-  if (
-    pathStr.includes('"') ||
-    pathStr.includes("'") ||
-    pathStr.includes('...')
-  ) {
+  // Check for double quotes or ellipses in "paths" - almost always a misinterpretation if not a very short name.
+  // We removed single quotes from this list to support users with apostrophes in their home directories.
+  if (pathStr.includes('"') || pathStr.includes('...')) {
     if (pathStr.length > 20) {
       return {
         isValid: false,
         error:
-          'Path contains suspicious characters (quotes or ellipses) and is too long to be a simple filename.',
+          'Path contains suspicious characters (double quotes or ellipses) and is too long to be a simple filename.',
       };
     }
   }