dean review

ethanndickson · ethanndickson · commit 39aa907f0d92 · 2025-02-28T06:11:39.000Z
diff --git a/agent/ls.go b/agent/ls.go
@@ -18,7 +18,7 @@ import (
 func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) {
 	ctx := r.Context()
 
-	var query LSQuery
+	var query LSRequest
 	if !httpapi.Read(ctx, rw, r, &query) {
 		return
 	}
@@ -45,7 +45,7 @@ func (*agent) HandleLS(rw http.ResponseWriter, r *http.Request) {
 	httpapi.Write(ctx, rw, http.StatusOK, resp)
 }
 
-func listFiles(query LSQuery) (LSResponse, error) {
+func listFiles(query LSRequest) (LSResponse, error) {
 	var fullPath []string
 	switch query.Relativity {
 	case LSRelativityHome:
@@ -67,28 +67,29 @@ func listFiles(query LSQuery) (LSResponse, error) {
 	}
 
 	fullPath = append(fullPath, query.Path...)
-	absolutePathString, err := filepath.Abs(filepath.Join(fullPath...))
+	fullPathRelative := filepath.Join(fullPath...)
+	absolutePathString, err := filepath.Abs(fullPathRelative)
 	if err != nil {
-		return LSResponse{}, xerrors.Errorf("failed to get absolute path: %w", err)
+		return LSResponse{}, xerrors.Errorf("failed to get absolute path of %q: %w", fullPathRelative, err)
 	}
 
 	f, err := os.Open(absolutePathString)
 	if err != nil {
-		return LSResponse{}, xerrors.Errorf("failed to open directory: %w", err)
+		return LSResponse{}, xerrors.Errorf("failed to open directory %q: %w", absolutePathString, err)
 	}
 	defer f.Close()
 
 	stat, err := f.Stat()
 	if err != nil {
-		return LSResponse{}, xerrors.Errorf("failed to stat directory: %w", err)
+		return LSResponse{}, xerrors.Errorf("failed to stat directory %q: %w", absolutePathString, err)
 	}
 
 	if !stat.IsDir() {
-		return LSResponse{}, xerrors.New("path is not a directory")
+		return LSResponse{}, xerrors.Errorf("path %q is not a directory", absolutePathString)
 	}
 
 	// `contents` may be partially populated even if the operation fails midway.
-	contents, _ := f.Readdir(-1)
+	contents, _ := f.ReadDir(-1)
 	respContents := make([]LSFile, 0, len(contents))
 	for _, file := range contents {
 		respContents = append(respContents, LSFile{
@@ -140,7 +141,7 @@ func pathToArray(path string) []string {
 	return out
 }
 
-type LSQuery struct {
+type LSRequest struct {
 	// e.g. [], ["repos", "coder"],
 	Path []string `json:"path"`
 	// Whether the supplied path is relative to the user's home directory,
diff --git a/agent/ls_internal_test.go b/agent/ls_internal_test.go
@@ -12,7 +12,7 @@ import (
 func TestListFilesNonExistentDirectory(t *testing.T) {
 	t.Parallel()
 
-	query := LSQuery{
+	query := LSRequest{
 		Path:       []string{"idontexist"},
 		Relativity: LSRelativityHome,
 	}
@@ -39,7 +39,7 @@ func TestListFilesPermissionDenied(t *testing.T) {
 	rel, err := filepath.Rel(home, reposDir)
 	require.NoError(t, err)
 
-	query := LSQuery{
+	query := LSRequest{
 		Path:       pathToArray(rel),
 		Relativity: LSRelativityHome,
 	}
@@ -62,12 +62,12 @@ func TestListFilesNotADirectory(t *testing.T) {
 	rel, err := filepath.Rel(home, filePath)
 	require.NoError(t, err)
 
-	query := LSQuery{
+	query := LSRequest{
 		Path:       pathToArray(rel),
 		Relativity: LSRelativityHome,
 	}
 	_, err = listFiles(query)
-	require.ErrorContains(t, err, "path is not a directory")
+	require.ErrorContains(t, err, "is not a directory")
 }
 
 func TestListFilesSuccess(t *testing.T) {
@@ -125,7 +125,7 @@ func TestListFilesSuccess(t *testing.T) {
 				queryComponents = pathToArray(rel)
 			}
 
-			query := LSQuery{
+			query := LSRequest{
 				Path:       queryComponents,
 				Relativity: tc.relativity,
 			}
@@ -161,7 +161,7 @@ func TestListFilesListDrives(t *testing.T) {
 		t.Skip("skipping test on non-Windows OS")
 	}
 
-	query := LSQuery{
+	query := LSRequest{
 		Path:       []string{},
 		Relativity: LSRelativityRoot,
 	}
@@ -173,14 +173,14 @@ func TestListFilesListDrives(t *testing.T) {
 		IsDir:              true,
 	})
 
-	query = LSQuery{
+	query = LSRequest{
 		Path:       []string{"C:\\"},
 		Relativity: LSRelativityRoot,
 	}
 	resp, err = listFiles(query)
 	require.NoError(t, err)
 
-	query = LSQuery{
+	query = LSRequest{
 		Path:       resp.AbsolutePath,
 		Relativity: LSRelativityRoot,
 	}
