CR feedback

Co-Authored-By: SteveSandersonMS <[email protected]>

Author: SteveSandersonMS

dotnet/test/SessionFsTests.cs

@@ -121,13 +121,7 @@ public async Task Should_Reject_SetProvider_When_Sessions_Already_Exist()
             }
             finally
             {
-                try
-                {
-                    await client2.ForceStopAsync();
-                }
-                catch
-                {
-                }
+                await client2.ForceStopAsync();
             }
         }
         finally
@@ -255,15 +249,18 @@ private CopilotClient CreateSessionFsClient(string providerRoot, bool useStdio =
     }
 
     private static string CreateProviderRoot()
-        => Path.Combine(Path.GetTempPath(), $"copilot-sessionfs-{Guid.NewGuid():N}");
+        => Path.Join(Path.GetTempPath(), $"copilot-sessionfs-{Guid.NewGuid():N}");
 
     private static string GetStoredPath(string providerRoot, string sessionId, string sessionPath)
     {
+        var safeSessionId = NormalizeRelativePathSegment(sessionId, nameof(sessionId));
         var relativeSegments = sessionPath
             .TrimStart('/', '\\')
-            .Split(['/', '\\'], StringSplitOptions.RemoveEmptyEntries);
+            .Split(['/', '\\'], StringSplitOptions.RemoveEmptyEntries)
+            .Select(segment => NormalizeRelativePathSegment(segment, nameof(sessionPath)))
+            .ToArray();
 
-        return Path.Combine([providerRoot, sessionId, .. relativeSegments]);
+        return Path.Join([providerRoot, safeSessionId, .. relativeSegments]);
     }
 
     private static async Task WaitForConditionAsync(Func<bool> condition, TimeSpan? timeout = null)
@@ -274,6 +271,7 @@ private static async Task WaitForConditionAsync(Func<bool> condition, TimeSpan?
     private static async Task WaitForConditionAsync(Func<Task<bool>> condition, TimeSpan? timeout = null)
     {
         var deadline = DateTime.UtcNow + (timeout ?? TimeSpan.FromSeconds(30));
+        Exception? lastException = null;
         while (DateTime.UtcNow < deadline)
         {
             try
@@ -283,17 +281,19 @@ private static async Task WaitForConditionAsync(Func<Task<bool>> condition, Time
                     return;
                 }
             }
-            catch (IOException)
+            catch (IOException ex)
             {
+                lastException = ex;
             }
-            catch (UnauthorizedAccessException)
+            catch (UnauthorizedAccessException ex)
             {
+                lastException = ex;
             }
 
             await Task.Delay(100);
         }
 
-        throw new TimeoutException("Timed out waiting for condition.");
+        throw new TimeoutException("Timed out waiting for condition.", lastException);
     }
 
     private static async Task<string> ReadAllTextSharedAsync(string path, CancellationToken cancellationToken = default)
@@ -305,16 +305,26 @@ private static async Task<string> ReadAllTextSharedAsync(string path, Cancellati
 
     private static void TryDeleteDirectory(string path)
     {
-        try
+        if (Directory.Exists(path))
         {
-            if (Directory.Exists(path))
-            {
-                Directory.Delete(path, recursive: true);
-            }
+            Directory.Delete(path, recursive: true);
         }
-        catch
+    }
+
+    private static string NormalizeRelativePathSegment(string segment, string paramName)
+    {
+        if (string.IsNullOrWhiteSpace(segment))
         {
+            throw new InvalidOperationException($"{paramName} must not be empty.");
         }
+
+        var normalized = segment.TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar);
+        if (Path.IsPathRooted(normalized) || normalized.Contains(Path.VolumeSeparatorChar))
+        {
+            throw new InvalidOperationException($"{paramName} must be a relative path segment: {segment}");
+        }
+
+        return normalized;
     }
 
     private sealed class TestSessionFsHandler(string sessionId, string rootDir) : ISessionFsHandler
@@ -456,12 +466,15 @@ public Task RenameAsync(SessionFsRenameParams request, CancellationToken cancell
 
         private string ResolvePath(string sessionPath)
         {
-            var sessionRoot = Path.GetFullPath(Path.Combine(rootDir, sessionId));
+            var normalizedSessionId = NormalizeRelativePathSegment(sessionId, nameof(sessionId));
+            var sessionRoot = Path.GetFullPath(Path.Join(rootDir, normalizedSessionId));
             var relativeSegments = sessionPath
                 .TrimStart('/', '\\')
-                .Split(['/', '\\'], StringSplitOptions.RemoveEmptyEntries);
+                .Split(['/', '\\'], StringSplitOptions.RemoveEmptyEntries)
+                .Select(segment => NormalizeRelativePathSegment(segment, nameof(sessionPath)))
+                .ToArray();
 
-            var fullPath = Path.GetFullPath(Path.Combine([sessionRoot, .. relativeSegments]));
+            var fullPath = Path.GetFullPath(Path.Join([sessionRoot, .. relativeSegments]));
             if (!fullPath.StartsWith(sessionRoot, StringComparison.Ordinal))
             {
                 throw new InvalidOperationException($"Path escapes session root: {sessionPath}");

python/copilot/generated/rpc.py

@@ -4068,25 +4068,25 @@ async def log(self, params: SessionLogParams, *, timeout: float | None = None) -
 
 class SessionFsHandler(Protocol):
     async def read_file(self, params: SessionFSReadFileParams) -> SessionFSReadFileResult:
-        ...
+        pass
     async def write_file(self, params: SessionFSWriteFileParams) -> None:
-        ...
+        pass
     async def append_file(self, params: SessionFSAppendFileParams) -> None:
-        ...
+        pass
     async def exists(self, params: SessionFSExistsParams) -> SessionFSExistsResult:
-        ...
+        pass
     async def stat(self, params: SessionFSStatParams) -> SessionFSStatResult:
-        ...
+        pass
     async def mkdir(self, params: SessionFSMkdirParams) -> None:
-        ...
+        pass
     async def readdir(self, params: SessionFSReaddirParams) -> SessionFSReaddirResult:
-        ...
+        pass
     async def readdir_with_types(self, params: SessionFSReaddirWithTypesParams) -> SessionFSReaddirWithTypesResult:
-        ...
+        pass
     async def rm(self, params: SessionFSRmParams) -> None:
-        ...
+        pass
     async def rename(self, params: SessionFSRenameParams) -> None:
-        ...
+        pass
 
 @dataclass
 class ClientSessionApiHandlers:

scripts/codegen/python.ts

@@ -509,7 +509,7 @@ function emitClientSessionHandlerMethod(
     if (method.stability === "experimental" && !groupExperimental) {
         lines.push(`        """.. warning:: This API is experimental and may change or be removed in future versions."""`);
     }
-    lines.push(`        ...`);
+    lines.push(`        pass`);
 }
 
 function emitClientSessionRegistrationMethod(