Skip to content

Commit dd4e76e

Browse files
authored
Merge pull request #1260
* KG-583 fix directory collapse in ListDirectoryUtil.kt * KG-583 fix test after rebase
1 parent 9fea02f commit dd4e76e

File tree

3 files changed

+100
-26
lines changed

3 files changed

+100
-26
lines changed

agents/agents-ext/src/commonMain/kotlin/ai/koog/agents/ext/tool/file/ListDirectoryUtil.kt

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,6 @@ private suspend fun <Path> buildNode(
6363
matchesFilter(fs, rootPath, childPath, filter)
6464
}
6565

66-
// At max depth with multiple children: return folder entry without children (collapsed)
67-
if (depth == 1 && visibleChildren.size > 1) {
68-
return buildFolderEntry(fs, currentPath, metadata, entries = null)
69-
}
70-
7166
val entries = mutableListOf<FileSystemEntry>()
7267
for ((childPath, childMeta) in visibleChildren) {
7368
when (childMeta.type) {
@@ -82,6 +77,8 @@ private suspend fun <Path> buildNode(
8277
if (nextDepth > 0) {
8378
buildNode(fs, rootPath, childPath, childMeta, nextDepth, filter)
8479
?.let { entries += it }
80+
} else if (matchesFilter(fs, rootPath, childPath, filter)) {
81+
entries += buildFolderEntry(fs, childPath, childMeta, entries = null)
8582
}
8683
}
8784
}

agents/agents-ext/src/jvmTest/kotlin/ai/koog/agents/ext/tool/file/ListDirectoryToolJvmTest.kt

Lines changed: 79 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class ListDirectoryToolJvmTest {
137137
}
138138

139139
@Test
140-
fun `multiple directories shows root only with depth 1`() = runBlocking {
140+
fun `multiple directories shows direct contents with depth 1`() = runBlocking {
141141
// Structure:
142142
// root/
143143
// ├── src/
@@ -148,8 +148,15 @@ class ListDirectoryToolJvmTest {
148148

149149
val resultText = tool.encodeResultToString(list(root, depth = 1))
150150

151-
// Expected: /path/to/root/
152-
val expectedText = "${root.toAbsolutePath().toString().norm()}/"
151+
// Expected:
152+
// /path/to/root/
153+
// src/
154+
// test/
155+
val expectedText = """
156+
${root.toAbsolutePath().toString().norm()}/
157+
src/
158+
test/
159+
""".trimIndent()
153160

154161
assertEquals(expectedText, resultText)
155162
}
@@ -177,7 +184,7 @@ class ListDirectoryToolJvmTest {
177184
}
178185

179186
@Test
180-
fun `unwrapping stops at multiple files with depth 1`() = runBlocking {
187+
fun `unwrapping shows multiple files with depth 1`() = runBlocking {
181188
// Structure:
182189
// project/
183190
// └── src/
@@ -194,13 +201,20 @@ class ListDirectoryToolJvmTest {
194201

195202
val resultText = tool.encodeResultToString(list(project, depth = 1))
196203

197-
// Expected: /path/to/project/src/main/kotlin/
198-
val expectedText = "${kotlin.toAbsolutePath().toString().norm()}/"
204+
// Expected:
205+
// /path/to/project/src/main/kotlin/
206+
// Main.kt (<0.1 KiB, 1 line)
207+
// Utils.kt (<0.1 KiB, 1 line)
208+
val expectedText = """
209+
${kotlin.toAbsolutePath().toString().norm()}/
210+
Main.kt (<0.1 KiB, 1 line)
211+
Utils.kt (<0.1 KiB, 1 line)
212+
""".trimIndent()
199213
assertEquals(expectedText, resultText)
200214
}
201215

202216
@Test
203-
fun `unwrapping stops at mixed files and directories`() = runBlocking {
217+
fun `unwrapping shows mixed files and directories`() = runBlocking {
204218
// Structure:
205219
// project/
206220
// └── src/
@@ -217,13 +231,20 @@ class ListDirectoryToolJvmTest {
217231

218232
val resultText = tool.encodeResultToString(list(project, depth = 1))
219233

220-
// Expected: /path/to/project/src/main/kotlin/
221-
val expectedText = "${kotlin.toAbsolutePath().toString().norm()}/"
234+
// Expected:
235+
// /path/to/project/src/main/kotlin/
236+
// Main.kt (<0.1 KiB, 1 line)
237+
// utils/
238+
val expectedText = """
239+
${kotlin.toAbsolutePath().toString().norm()}/
240+
Main.kt (<0.1 KiB, 1 line)
241+
utils/
242+
""".trimIndent()
222243
assertEquals(expectedText, resultText)
223244
}
224245

225246
@Test
226-
fun `multiple entries at root level prevents unwrapping`() = runBlocking {
247+
fun `multiple entries at root level shows direct contents`() = runBlocking {
227248
// Structure:
228249
// project/
229250
// ├── README.md
@@ -242,8 +263,15 @@ class ListDirectoryToolJvmTest {
242263

243264
val resultText = tool.encodeResultToString(list(project, depth = 1))
244265

245-
// Expected: /path/to/project/
246-
val expectedText = "${project.toAbsolutePath().toString().norm()}/"
266+
// Expected:
267+
// /path/to/project/
268+
// README.md (<0.1 KiB, 1 line)
269+
// src/
270+
val expectedText = """
271+
${project.toAbsolutePath().toString().norm()}/
272+
README.md (<0.1 KiB, 1 line)
273+
src/
274+
""".trimIndent()
247275

248276
assertEquals(expectedText, resultText)
249277
}
@@ -461,20 +489,24 @@ class ListDirectoryToolJvmTest {
461489
// README.md (<0.1 KiB, 1 line)
462490
// src/
463491
// main/kotlin/com/example/
492+
// Main.kt (<0.1 KiB, 1 line)
493+
// Utils.kt (<0.1 KiB, 1 line)
464494
// test/TestUtils.kt (<0.1 KiB, 1 line)
465495
val expectedText = """
466496
${project.toAbsolutePath().toString().norm()}/
467497
README.md (<0.1 KiB, 1 line)
468498
src/
469499
main/kotlin/com/example/
500+
Main.kt (<0.1 KiB, 1 line)
501+
Utils.kt (<0.1 KiB, 1 line)
470502
test/TestUtils.kt (<0.1 KiB, 1 line)
471503
""".trimIndent()
472504

473505
assertEquals(expectedText, resultText)
474506
}
475507

476508
@Test
477-
fun `unwrapping stops at first branching point`() = runBlocking {
509+
fun `unwrapping shows branches at first branching point`() = runBlocking {
478510
// Structure:
479511
// project/
480512
// └── a/
@@ -489,8 +521,40 @@ class ListDirectoryToolJvmTest {
489521

490522
val resultText = tool.encodeResultToString(list(project, depth = 1))
491523

492-
// Expected: /path/to/project/a/b/
493-
val expectedText = "${b.toAbsolutePath().toString().norm()}/"
524+
// Expected:
525+
// /path/to/project/a/b/
526+
// c1/
527+
// c2/
528+
val expectedText = """
529+
${b.toAbsolutePath().toString().norm()}/
530+
c1/
531+
c2/
532+
""".trimIndent()
533+
534+
assertEquals(expectedText, resultText)
535+
}
536+
537+
@Test
538+
fun `list shows direct children`() = runBlocking {
539+
// Structure:
540+
// project/
541+
// ├── LICENSE.txt
542+
// └── README.md
543+
val dir = createDir("project-depth1")
544+
dir.resolve("README.md").createFile().writeText("hello") // 1 line
545+
dir.resolve("LICENSE.txt").createFile().writeText("MIT") // 1 line
546+
547+
val resultText = tool.encodeResultToString(list(dir, depth = 1))
548+
549+
// Expected:
550+
// /path/to/project-depth1/
551+
// LICENSE.txt (<0.1 KiB, 1 line)
552+
// README.md (<0.1 KiB, 1 line)
553+
val expectedText = """
554+
${dir.toAbsolutePath().toString().norm()}/
555+
LICENSE.txt (<0.1 KiB, 1 line)
556+
README.md (<0.1 KiB, 1 line)
557+
""".trimIndent()
494558

495559
assertEquals(expectedText, resultText)
496560
}

agents/agents-ext/src/jvmTest/kotlin/ai/koog/agents/ext/tool/file/ListDirectoryUtilJvmTest.kt

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class ListDirectoryUtilJvmTest {
4545
}
4646

4747
@Test
48-
fun `buildDirectoryTree returns folder with no entries when depth 1 and multiple children`() = runTest {
48+
fun `buildDirectoryTree returns folder with entries when depth 1 and multiple children`() = runTest {
4949
// Structure:
5050
// dir/
5151
// ├── f1.txt
@@ -59,7 +59,10 @@ class ListDirectoryUtilJvmTest {
5959

6060
assertIs<FileSystemEntry.Folder>(entry)
6161
assertEquals(d.toAbsolutePath().toString(), entry.path)
62-
assertNull(entry.entries)
62+
val entries = assertNotNull(entry.entries)
63+
assertEquals(2, entries.size)
64+
val names = entries.map { (it as FileSystemEntry.File).name }.toSet()
65+
assertEquals(setOf("f1.txt", "f2.txt"), names)
6366
}
6467

6568
@Test
@@ -157,9 +160,12 @@ class ListDirectoryUtilJvmTest {
157160
val entry = buildDirectoryTree(fs, level1, metadata, maxDepth = 1) as FileSystemEntry.Folder
158161

159162
// level1 has 1 child (level2 directory), so it unwraps
160-
// level2 has multiple children at maxDepth=1, so its entries should be null
163+
// level2 has multiple children - with fixed behavior, they are listed
161164
val level2Folder = assertNotNull(entry.entries).single() as FileSystemEntry.Folder
162-
assertNull(level2Folder.entries)
165+
val level2Entries = assertNotNull(level2Folder.entries)
166+
assertEquals(2, level2Entries.size)
167+
val names = level2Entries.map { (it as FileSystemEntry.File).name }.toSet()
168+
assertEquals(setOf("file1.txt", "file2.txt"), names)
163169
}
164170

165171
@Test
@@ -179,8 +185,15 @@ class ListDirectoryUtilJvmTest {
179185
val metadata = assertNotNull(fs.metadata(level1))
180186
val entry = buildDirectoryTree(fs, level1, metadata, maxDepth = 1) as FileSystemEntry.Folder
181187

182-
// level1 has multiple children at maxDepth=1, so entries should be null
183-
assertNull(entry.entries)
188+
// level1 has multiple children - with fixed behavior, they are listed as collapsed directories
189+
val entries = assertNotNull(entry.entries)
190+
assertEquals(2, entries.size)
191+
val names = entries.map { (it as FileSystemEntry.Folder).name }.toSet()
192+
assertEquals(setOf("dir1", "dir2"), names)
193+
// Child directories should be collapsed (entries = null) since we're at the depth limit
194+
entries.forEach { dir ->
195+
assertNull((dir as FileSystemEntry.Folder).entries)
196+
}
184197
}
185198

186199
@Test

0 commit comments

Comments
 (0)