xiuper-fs: SQLite shell parity + MCP filesystem integration test#520
xiuper-fs: SQLite shell parity + MCP filesystem integration test#520
Conversation
Fix state recomposition by reading State.value and using stable key order. This ensures Compose properly tracks state changes and maintains deterministic hook ordering across recompositions. Also removes trailing whitespace in tests.
…ework - Make conformance tests capability-aware (detect supportsMkdir/Delete) - PosixSubsetConformanceTest: skip unsupported ops or assert ENOTSUP - PosixSubsetConformanceRunner: early return for unsupported backends - DbPosixSubsetConformanceTest: mirrored capability awareness - Declare DbFsBackend as CapabilityAwareBackend (supportsMkdir/Delete = true) - Add migration framework: - Migration interface + MigrationRegistry (path discovery) - createDatabase(driver) overload for PRAGMA user_version tracking - Migration_1_to_2 example (xattr table, commented out) - MigrationTest: fresh DB, up-to-date, future version, registry tests - Document strategy in docs/filesystem/xiuper-fs-db-migration-design.md All tests pass (JVM/JS); migration registry currently empty (v1 is current schema). Backend capabilities now allow REST-FS to declare unsupported operations cleanly.
…prehensive tests - Enable Migration_1_to_2 with FsXattr table (path+name composite PK, CASCADE DELETE) - Add SQLDelight migration file (1.sqm) to upgrade existing databases - Update FsNode.sq schema to v2 with extended attributes table - Add MigrationUpgradeTest for data integrity verification across versions - Fix MigrationTest to handle non-empty registry (test v2→v3 path instead) - Add RestFsConformanceTest to verify capability-aware behavior - All migration tests passing (8/8), REST backend correctly declares read-only capabilities Platforms: JVM/Android/iOS (real SQLite), WASM (sql.js), JS/Node (explicit unsupported)
**Policy & Security:** - Add MountPolicy interface for access control (AllowAll, ReadOnly presets) - Integrate policy checks into XiuperVfs (write/delete approval hooks) - Support read-only mounts via existing Mount.readOnly flag **Audit System:** - Add FsAuditEvent for operation tracking (operation, path, backend, status, latency) - Add FsAuditCollector interface (NoOp, Console implementations) - Integrate audit into XiuperVfs withAudit() wrapper for all operations **Compose Integration:** - Add FsRepository adapter for reactive filesystem access - Provide StateFlow-based observeDir/observeFile/observeText methods - Support manual refresh and cache invalidation - Automatic parent directory refresh on write/delete All tests passing (migration + conformance + core VFS)
Covers: - Feature overview (Core VFS, Backends, Migration, Security, Compose) - Platform support matrix - Quick start examples - Architecture diagram - Testing commands - Future roadmap (REST-FS advanced, MCP backend, Advanced policies) All major features documented: ✅ VFS infrastructure + mount router ✅ REST/DB/InMemory backends ✅ Migration framework v1→v2 ✅ Policy & audit system ✅ Compose integration (FsRepository) ✅ Capability-aware conformance tests
Add new MCP (Model Context Protocol) backend with supporting tests and REST FS enhancements. Includes advanced REST FS implementation and updated mount policies.
Add a new shell filesystem interpreter with built-in commands and GitHub filesystem support. Includes implementation, tests, and example usage.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded a PhoneAndroid icon and UI improvements; fixed Compose state observation ordering; and introduced a new xiuper-fs Kotlin Multiplatform module providing a virtual filesystem (multiple backends, policies, audit, SQLDelight persistence), shell interpreter, Compose integration, drivers, migrations, examples, and a broad test suite. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Shell as ShellFsInterpreter
participant VFS as XiuperVfs
participant Policy as MountPolicy
participant Backend as FsBackend
participant Audit as FsAuditCollector
User->>Shell: execute(cmd)
Shell->>Shell: parseCommand(), resolvePath()
Shell->>VFS: perform operation (stat/list/read/write/...)
VFS->>VFS: find mount by longest prefix
VFS->>Policy: checkOperation(op, path)
Policy-->>VFS: allow / FsException
VFS->>Audit: start timing
VFS->>Backend: dispatch op(path, ...)
Backend-->>VFS: result or throw FsException
VFS->>Audit: collect event(status/latency)
VFS-->>Shell: return result or propagate error
Shell-->>User: format and return stdout/stderr/exitCode
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Augment PR SummarySummary: This PR introduces a new Kotlin Multiplatform virtual filesystem module and expands NanoDSL rendering UX. Changes:
Technical Notes: MCP integration testing is gated behind 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| if (node.isDir != 0L) { | ||
| val anyChild = database.fsNodeQueries | ||
| .selectChildren(normalized.value + "/%") | ||
| .executeAsOneOrNull() |
There was a problem hiding this comment.
selectChildren(...).executeAsOneOrNull() can throw when a directory has >1 child (because the query returns multiple rows), so delete() may crash instead of returning ENOTEMPTY. Consider checking for “any child exists” in a way that can’t raise a multi-row exception (e.g., limit the query to 1 row).
🤖 Was this useful? React with 👍 or 👎
|
|
||
| path.value.startsWith("/resources/") && !path.value.endsWith("/") -> { | ||
| val uri = path.value.removePrefix("/resources/") | ||
| val resource = getResource(uri) |
There was a problem hiding this comment.
list("/resources") exposes resource names via encodeUri(resource.uri), but stat() looks up getResource(uri) without decoding, so stat() on a listed resource will typically return ENOENT. It seems stat() should apply the same decoding as read() to keep the directory listing contract consistent.
🤖 Was this useful? React with 👍 or 👎
| private val baseUrl: String = "https://api.github.com" | ||
| ) : FsBackend { | ||
|
|
||
| private val client = HttpClient { |
There was a problem hiding this comment.
This backend owns a Ktor HttpClient but doesn’t expose any way to close it; in long-lived IntelliJ/plugin contexts this can leak resources/threads. Consider making the backend lifecycle-aware (or injecting a shared client) so it can be disposed cleanly. (Guideline: no_memory_leaks)
🤖 Was this useful? React with 👍 or 👎
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive SQLite-backed database filesystem (DbFsBackend) and MCP (Model Context Protocol) integration to the xiuper-fs virtual filesystem module, with extensive testing and platform support.
Key Changes:
- Database migration framework with PRAGMA user_version tracking and v1→v2 migration path (FsXattr table)
- Shell interpreter for POSIX-like commands (mkdir/echo/cat/ls/cp/mv/rm) over any FsBackend
- MCP filesystem backend with resource/tool mapping via official Kotlin SDK
- Platform-specific database drivers (JVM/Android/iOS/WASM/JS with noop)
- Comprehensive conformance testing suite with capability-aware tests
Reviewed changes
Copilot reviewed 56 out of 57 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| xiuper-fs/build.gradle.kts | New module configuration with SQLDelight and MCP SDK dependencies |
| xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/* | Core VFS types and interfaces (FsPath, FsTypes, XiuperFileSystem) |
| xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/* | DbFsBackend and migration framework implementation |
| xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/* | Shell interpreter with builtin commands |
| xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/* | MCP backend for protocol resource/tool mapping |
| xiuper-fs/src/jvmTest/kotlin/* | Integration tests for DB shell smoke tests and MCP filesystem |
| mpp-ui/src/commonMain/kotlin/.../StatefulNanoRenderer.kt | UI state tracking fix with sorted key observation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // IMPORTANT: we must *read* the collected State's `.value` so Compose tracks it. | ||
| // Use a stable key order to keep hook ordering deterministic across recompositions. | ||
| val observedKeys = remember(runtime) { runtime.declaredKeys.toList().sorted() } | ||
| observedKeys.forEach { key -> | ||
| runtime.state.flow(key).collectAsState().value |
There was a problem hiding this comment.
The comment mentions reading .value to ensure Compose tracks the state, but this approach could cause unnecessary reads during composition. Consider whether all declared keys need to be read every render, as this could impact performance when there are many state keys. It might be more efficient to only observe keys that are actually used in the current render tree.
| */ | ||
| fun createDatabase(driver: SqlDriver): XiuperFsDatabase { | ||
| val currentVersion = getUserVersion(driver) | ||
| val targetVersion = XiuperFsDatabase.Schema.version.toInt() |
There was a problem hiding this comment.
The database schema version is hardcoded to 2 in XiuperFsDatabase.Schema.version, but this appears to be generated code from SQLDelight. Ensure that this version value is synchronized with the MigrationRegistry and that the Schema.version correctly reflects the current target schema version (v2). If Schema.version returns 1, migrations will never be applied.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt (1)
241-261: Replace hardcoded colors with theme tokens.Per coding guidelines: "Never use hardcoded colors - always use design tokens from the design system" and "Use
AutoDevColorsorMaterialTheme.colorSchemefor Compose styling."The preview background and border colors should use
MaterialTheme.colorSchemeinstead of hardcoded hex values.🔎 Suggested fix using theme colors
val backgroundColor = if (isDarkTheme) { - Color(0xFF1E1E1E) // Dark background + MaterialTheme.colorScheme.surfaceContainerLowest } else { - Color(0xFFF5F5F5) // Light background + MaterialTheme.colorScheme.surfaceContainerHigh } Box( modifier = previewModifier .background( color = backgroundColor, shape = RoundedCornerShape(8.dp) ) .border( width = 1.dp, color = if (isDarkTheme) { - Color(0xFF3C3C3C) + MaterialTheme.colorScheme.outlineVariant } else { - Color(0xFFE0E0E0) + MaterialTheme.colorScheme.outline.copy(alpha = 0.5f) }, shape = RoundedCornerShape(8.dp) )
🧹 Nitpick comments (46)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt (2)
70-70: Approve zoom functionality with minor precision note.The zoom implementation is clean. Consider rounding the zoom level to avoid floating-point drift after repeated increments/decrements:
🔎 Optional: Round zoom level to avoid precision drift
- onClick = { if (zoomLevel > 0.5f) zoomLevel -= 0.1f } + onClick = { if (zoomLevel > 0.5f) zoomLevel = (zoomLevel - 0.1f).coerceIn(0.5f, 2.0f).let { (it * 10).toInt() / 10f } } ... - onClick = { if (zoomLevel < 2.0f) zoomLevel += 0.1f } + onClick = { if (zoomLevel < 2.0f) zoomLevel = (zoomLevel + 0.1f).coerceIn(0.5f, 2.0f).let { (it * 10).toInt() / 10f } }Also applies to: 172-186
314-370: Consider removing unusedNanoDSLToggleButton.This private composable appears unused after all toggle buttons were migrated to
NanoDSLIconButton. If it's no longer needed, consider removing it to reduce code surface area.xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackendTest.kt (1)
38-53: Test creates unconnected MCP client.The test helper creates a
Clientwithout callingconnect(). While sufficient for basic construction testing, any backend operations that interact with the MCP server (list, read, write) will fail since the client is not connected to a transport.Consider adding a comment clarifying that this test only validates construction, not functional operations, since the documentation suggests production usage requires
client.connect(transport).🔎 Clarify test scope with a comment:
private fun createTestBackend(): McpBackend { // For real testing, you would need to: // 1. Launch an actual MCP server process // 2. Create a Client with proper Transport // 3. Connect the client // // Example (requires actual MCP server): // val client = Client(clientInfo = Implementation(name = "Test", version = "1.0.0")) // val transport = StdioClientTransport(input, output) // client.connect(transport) // return DefaultMcpBackend(client) - // For now, create a simple backend that would work with a real client + // For now, create a simple backend for construction testing only. + // Note: This client is NOT connected, so backend operations will fail. val client = Client(clientInfo = Implementation(name = "TestClient", version = "1.0.0")) return DefaultMcpBackend(client) }xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpFilesystemServerIntegrationTest.kt (2)
58-58: Consider reducing timeout for faster CI feedback.The 120-second timeout is quite long for a test. While it may be necessary for npx package installation and server startup, consider whether a shorter timeout (e.g., 30-60 seconds) would be sufficient after the first run when packages are cached.
95-101: Consider validating file content in read_file test.The test asserts that
read_filesucceeds and returns a non-null message but doesn't validate the actual content. Since you wrote "hello from mcp" to the file, you could strengthen this test by checking for that content in the result.🔎 Example assertion:
assertTrue(readResult.ok, "read_file should succeed") assertNotNull(readResult.message) + assertTrue( + readResult.message?.contains("hello from mcp") == true, + "read_file should return file content" + )xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFsAdvanced.kt (1)
45-50: Acknowledged limitation: dir handler returns empty list.The comment correctly identifies that field listing requires async data loading, which isn't supported in the current dir handler architecture. This means
/fields/directories will appear empty when listed, even though individual field files are readable.This limitation may confuse users who expect to discover fields via directory listing. Consider documenting this behavior in the function's KDoc.
xiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt (1)
6-10: Consider using an absolute path or user directory for the database file.The relative path
"jdbc:sqlite:xiuper-fs.db"will create the database in the current working directory, which varies based on how the application is launched. For production use, consider:
- Using a well-known location like user home directory
- Accepting a configurable path
- Or documenting that callers should manage the working directory
💡 Example using user home directory
actual fun createDriver(): SqlDriver { val dbPath = System.getProperty("user.home") + "/.xiuper-fs/xiuper-fs.db" // Ensure directory exists return JdbcSqliteDriver("jdbc:sqlite:$dbPath") }xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackendTest.kt (1)
9-85: Consider making GitHub API tests opt-in integration tests.These tests make real HTTP calls to the GitHub API, which introduces several reliability concerns:
- Network dependency and potential flakiness
- GitHub API rate limiting (60 requests/hour for unauthenticated calls)
- CI/CD environments may lack internet access
- Reliance on third-party repository stability
The PR description mentions using
RUN_MCP_INTEGRATION_TESTS=truefor MCP integration tests. Consider applying a similar pattern here:@Test fun `list root repository contents`() = runTest { val shouldRun = System.getenv("RUN_GITHUB_INTEGRATION_TESTS")?.toBoolean() ?: false if (!shouldRun) return@runTest // existing test code... }Alternatively, consider:
- Moving these to a separate
integrationTestsource set- Using a mock HTTP server (like MockWebServer) for unit tests
- Keeping one smoke test as opt-in, and mocking the rest
xiuper-fs/src/wasmJsMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt (1)
15-21: Document bundler requirements for WASM JS Worker setup.The dynamic Worker creation relies on:
- ES module syntax (
import.meta.url)- Bundler resolution of
@cashapp/sqldelight-sqljs-worker- Browser environment
This will fail at runtime if:
- The bundler (webpack/vite/etc.) isn't configured to handle worker imports
- The
@cashapp/sqldelight-sqljs-workerpackage isn't installed- Code runs outside a browser context
Consider:
- Adding setup documentation to the README's Platform Support section for WASM
- Adding a runtime check with a clearer error message
- Providing a sample bundler configuration
📝 Example documentation addition for README.md
### WASM Setup Requirements For wasmJs target, ensure your bundler is configured to handle Web Workers: **Vite:** ```js // vite.config.js export default { worker: { format: 'es' } }Dependencies:
npm install @cashapp/sqldelight-sqljs-worker</details> </blockquote></details> <details> <summary>xiuper-fs/src/androidMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.android.kt (1)</summary><blockquote> `20-32`: **Consider using OkHttp engine for Android.** The CIO engine works on Android, but the OkHttp engine (`io.ktor:ktor-client-okhttp`) is typically preferred for Android as it integrates better with the Android networking stack and provides features like certificate pinning, connection pooling optimized for mobile, and better compatibility with Android's network security configuration. <details> <summary>🔎 Alternative using OkHttp engine:</summary> ```diff -import io.ktor.client.engine.cio.CIO +import io.ktor.client.engine.okhttp.OkHttp - return HttpClient(CIO) { + return HttpClient(OkHttp) {xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/policy/PolicyTest.kt (4)
51-59: UseassertTrue/assertFalsefrom kotlin.test instead ofassert().The
assert()function can be disabled at runtime and doesn't provide descriptive failure messages. For consistent test behavior and better diagnostics, prefer kotlin.test assertions.🔎 Apply this diff:
@Test fun pathPatternParsing() { val exact = PathPattern.parse("/exact/path") - assert(exact is PathPattern.Exact) + assertTrue(exact is PathPattern.Exact, "Expected Exact pattern") val prefix = PathPattern.parse("/prefix/") - assert(prefix is PathPattern.Prefix) + assertTrue(prefix is PathPattern.Prefix, "Expected Prefix pattern") val wildcard = PathPattern.parse("/path/*/file") - assert(wildcard is PathPattern.Wildcard) + assertTrue(wildcard is PathPattern.Wildcard, "Expected Wildcard pattern") }
62-69: Replaceassert()withassertTrue/assertFalsefor consistent test assertions.Same issue as above -
assert()should be replaced with kotlin.test assertions for reliable test behavior.🔎 Apply this diff:
@Test fun wildcardPatternMatching() { val pattern = PathPattern.Wildcard("/api/*/users") - assert(pattern.matches(FsPath("/api/v1/users"))) - assert(pattern.matches(FsPath("/api/v2/users"))) - assert(!pattern.matches(FsPath("/api/v1/posts"))) + assertTrue(pattern.matches(FsPath("/api/v1/users")), "Should match /api/v1/users") + assertTrue(pattern.matches(FsPath("/api/v2/users")), "Should match /api/v2/users") + assertFalse(pattern.matches(FsPath("/api/v1/posts")), "Should not match /api/v1/posts") }
86-92: Replaceassert()withassertTruefor consistent test assertions.🔎 Apply this diff:
- assert(approvalRequested) + assertTrue(approvalRequested, "Approval should have been requested") assertEquals("/important/file", pathRequested?.value) assertNotNull(error) assertEquals(FsErrorCode.EACCES, error.code)
115-118: Replaceassert()withassertFalseandassertNullfor consistent test assertions.🔎 Apply this diff:
// Read should not require approval val error = policy.checkOperation(FsOperation.READ, FsPath("/file")) - assert(!approvalRequested) + assertFalse(approvalRequested, "Approval should not be requested for READ") assertNull(error)xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationUpgradeTest.kt (2)
72-75: UseassertNotNullinstead ofassertTrue(... != null)for clearer assertions.This provides better failure messages and is more idiomatic.
🔎 Apply this diff:
assertEquals("user.comment", xattr.first) - assertTrue(xattr.second != null) + assertNotNull(xattr.second, "xattr value should not be null")
106-112: Simplify null-check assertions usingassertNotNullwith smart casts.Using
assertNotNullreturns the non-null value, enabling cleaner assertion chains.🔎 Apply this diff:
assertEquals(2, nodes.size) - val root = nodes.find { it.path == "/" } - assertTrue(root != null && root.isDir == 1L) + val root = assertNotNull(nodes.find { it.path == "/" }, "Root node should exist") + assertEquals(1L, root.isDir, "Root should be a directory") - val file = nodes.find { it.path == "/file.txt" } - assertTrue(file != null && file.isDir == 0L) + val file = assertNotNull(nodes.find { it.path == "/file.txt" }, "File node should exist") + assertEquals(0L, file.isDir, "File should not be a directory")xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/RestSchemaBuilderTest.kt (1)
26-42: Add import forWriteResultto avoid fully qualified name.The fully qualified
cc.unitmesh.xiuper.fs.WriteResulton line 32 could be simplified by adding an import.🔎 Apply this diff at the imports:
import cc.unitmesh.xiuper.fs.FsEntry import cc.unitmesh.xiuper.fs.FsPath import cc.unitmesh.xiuper.fs.ReadResult import cc.unitmesh.xiuper.fs.WriteOptions +import cc.unitmesh.xiuper.fs.WriteResultAnd update line 32:
- cc.unitmesh.xiuper.fs.WriteResult(ok = true) + WriteResult(ok = true)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellTypes.kt (1)
8-13: Consider using an immutable map forenvironmentto avoid copy() aliasing.
ShellContextis a data class, butenvironmentdefaults to aMutableMap. Whencopy()is called, the new instance shares the same mutable map reference, which can lead to unintended side effects if one copy modifies the map.🔎 Suggested change to use an immutable map:
data class ShellContext( val backend: FsBackend, val workingDirectory: String = "/", - val environment: MutableMap<String, String> = mutableMapOf(), + val environment: Map<String, String> = emptyMap(), val stdin: String? = null )If mutation is needed, callers can create a new map:
context.copy(environment = context.environment + ("KEY" to "value")).xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceTest.kt (1)
197-205: Consider extractingassertFsErrorto a shared test utility.This helper is duplicated in
DbPosixSubsetConformanceTest(lines 209-217). Extracting it to a shared test utility (e.g., incommonTest) would reduce duplication and ensure consistent error assertion behavior across conformance tests.xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.kt (1)
14-26: Redundant path normalization.
FsPath.of(path.value)is called to normalize a path that is already anFsPath. SinceFsPathenforces normalization at construction, this re-normalization is unnecessary. This pattern repeats inlist,read,write,mkdir, anddelete.🔎 Suggested simplification:
override suspend fun stat(path: FsPath): FsStat { - val normalized = FsPath.of(path.value) - if (normalized.value == "/") { + if (path.value == "/") { return FsStat(path = FsPath("/"), isDirectory = true) } - val node = database.fsNodeQueries.selectByPath(normalized.value).executeAsOneOrNull() - ?: throw FsException(FsErrorCode.ENOENT, "No such path: ${normalized.value}") + val node = database.fsNodeQueries.selectByPath(path.value).executeAsOneOrNull() + ?: throw FsException(FsErrorCode.ENOENT, "No such path: ${path.value}") val isDir = node.isDir != 0L - val size = if (isDir) null else (node.content?.size?.toLong() ?: 0L) - return FsStat(path = normalized, isDirectory = isDir, size = size) + val size = if (isDir) null else (node.content?.size?.toLong() ?: 0L) + return FsStat(path = path, isDirectory = isDir, size = size) }Apply similar changes to other methods.
xiuper-fs/build.gradle.kts (1)
118-119: Update sql.js to the latest version.sql.js is significantly outdated at 1.8.0. Consider updating to version 1.13.0, which includes performance improvements, updated SQLite core (version 3.49), and Emscripten 4.x. @cashapp/sqldelight-sqljs-worker at 2.1.0 is reasonably current (2.2.1 is latest), so no urgent change needed there, though updating both together would ensure compatibility.
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/migrations/Migration.kt (1)
26-31: Consider enforcing the documented ordering invariant.The doc comment states migrations are "ordered by fromVersion", but the list relies on manual ordering. If a future contributor adds migrations out of order, the
path()function will still work (since it usesfirstOrNull { it.fromVersion == version }), but the documentation would be misleading.Consider either:
- Explicitly sorting:
val all: List<Migration> = listOf(...).sortedBy { it.fromVersion }- Adding an
initblock that validates ordering with an assertionxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/FsAudit.kt (1)
41-45: Consider platform-appropriate logging for Console collector.Using
printlndirectly incommonMainworks but may not integrate well with platform-specific logging frameworks. For production use, consider making this anexpect/actualpattern or accepting a logging function parameter.For a reference implementation/example collector, this is acceptable as-is.
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt (1)
77-84: Prefer parameterized query forsetUserVersion.While
versionis anInt(not exploitable), string interpolation in SQL is an anti-pattern. SQLite PRAGMAs with dynamic values should use parameterized execution where possible.However, SQLite's
PRAGMA user_version = ?with parameter binding may not work on all drivers. If parameterization isn't supported for this PRAGMA, add a comment explaining why interpolation is safe here.🔎 Add defensive comment if parameterization isn't feasible:
private fun setUserVersion(driver: SqlDriver, version: Int) { + // Note: PRAGMA user_version requires literal value; parameterization not supported. + // Safe because version is Int, not user input. driver.execute( identifier = null, sql = "PRAGMA user_version = $version", parameters = 0, binders = null ) }xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/GitHubRestSchemaTest.kt (1)
26-32: Use type-safe assertions to avoid redundant smart casts.After
assertTrue(newEntry is FsEntry.Special), the subsequent cast(newEntry as FsEntry.Special)is redundant but necessary becauseassertTruedoesn't enable smart casting. Consider usingassertIsor pattern matching for cleaner assertions.🔎 Suggested refactor using assertIs (if available) or require:
val newEntry = entries.first { it.name == "new" } - assertTrue(newEntry is FsEntry.Special) - assertEquals(FsEntry.SpecialKind.MagicNew, (newEntry as FsEntry.Special).kind) + require(newEntry is FsEntry.Special) { "Expected Special entry for 'new'" } + assertEquals(FsEntry.SpecialKind.MagicNew, newEntry.kind) val queryEntry = entries.first { it.name == "query" } - assertTrue(queryEntry is FsEntry.Special) - assertEquals(FsEntry.SpecialKind.ControlQuery, (queryEntry as FsEntry.Special).kind) + require(queryEntry is FsEntry.Special) { "Expected Special entry for 'query'" } + assertEquals(FsEntry.SpecialKind.ControlQuery, queryEntry.kind)xiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubShellExample.kt (2)
144-148: Consider checkingcdcommand result in examples.The
cdcommand result is discarded. If the path doesn't exist, subsequent commands may behave unexpectedly. For a demo, this might be fine, but checking the exit code would make the example more robust and educational.🔎 Optional defensive check:
val repoPath = "/microsoft/vscode/main" - shell.execute("cd $repoPath") + val cdResult = shell.execute("cd $repoPath") + if (cdResult.exitCode != 0) { + println("Failed to cd to $repoPath: ${cdResult.stderr}") + return + }
189-198: Context printing may produce duplicates for adjacent matches.If the search pattern appears on consecutive lines, the context windows will overlap and print the same lines multiple times. For a demo this is minor, but worth noting.
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.kt (2)
54-73: Consider defensive copy on write as well.Similar to the read issue, if the caller retains a reference to
contentand modifies it afterwrite(), the internal state could be corrupted. For a reference implementation, defensive copying on both read and write ensures isolation.🔎 Apply this fix:
is Node.File -> { - existing.bytes = content + existing.bytes = content.copyOf() return WriteResult(ok = true) } null -> { - parentDir.children[name] = Node.File(content) + parentDir.children[name] = Node.File(content.copyOf()) return WriteResult(ok = true) }
5-12: Consider documenting thread-safety expectations.The class doc describes semantics well but doesn't mention thread-safety. Since this uses mutable collections without synchronization, document that it's not thread-safe or add synchronization if concurrent access is expected.
xiuper-fs/src/commonMain/sqldelight/cc/unitmesh/xiuper/fs/db/FsNode.sq (2)
37-41:selectChildrenLIKE pattern may match unintended paths.The LIKE query
WHERE path LIKE ?requires careful escaping. If the caller passes/foo/bar%, it would match children. However, paths containing%or_characters would need escaping, and this pattern also matches deeper descendants (e.g.,/foo/bar/baz/qux), not just immediate children.Consider using a more precise query for immediate children:
🔎 Suggested alternative for immediate children:
selectChildren: SELECT path, isDir, content, mtimeEpochMillis FROM FsNode -WHERE path LIKE ? +WHERE path LIKE ? AND path NOT LIKE ? || '/%/%' ORDER BY path;Or filter in application code after fetching with the LIKE pattern.
8-17: Missing queries for FsXattr operations.The
FsXattrtable is defined but no SQLDelight queries are provided for extended attribute operations (get, set, list, remove xattrs). If xattr operations are intended to be supported, consider adding queries:🔎 Suggested xattr queries:
getXattr: SELECT value FROM FsXattr WHERE path = ? AND name = ?; setXattr: INSERT OR REPLACE INTO FsXattr(path, name, value) VALUES (?, ?, ?); listXattrs: SELECT name FROM FsXattr WHERE path = ?; removeXattr: DELETE FROM FsXattr WHERE path = ? AND name = ?;xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationTest.kt (2)
100-107: Avoid SQL string interpolation; use parameterized query.Line 103 uses string interpolation (
"PRAGMA user_version = $version") which is a SQL injection anti-pattern. While this is test code with controlled input, it sets a bad example. SQLite PRAGMAs don't support parameterized values, but you can validate the input:🔎 Suggested defensive fix:
private fun setUserVersion(driver: SqlDriver, version: Int) { + require(version >= 0) { "Version must be non-negative" } driver.execute( identifier = null, sql = "PRAGMA user_version = $version", parameters = 0, binders = null ) }
53-57: Consider safer null handling in assertions.Using
exception.message!!(also on lines 64, 79) will throw NPE if the message is null, masking the actual test failure. Consider usingassertNotNullor safe access:🔎 Suggested fix:
val exception = assertFailsWith<IllegalStateException> { createDatabase(driver) } - assertTrue(exception.message!!.contains("newer than supported")) + assertTrue(exception.message?.contains("newer than supported") == true, + "Expected 'newer than supported' in: ${exception.message}")xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreter.kt (2)
130-137: Command parsing doesn't handle quoted arguments.The comment acknowledges this, but commands like
echo "hello world" > file.txtwill be incorrectly split into["echo", "\"hello", "world\"", ">", "file.txt"]instead of preserving the quoted string.🔎 Suggested approach:
private fun parseCommand(line: String): ParsedCommand { val parts = mutableListOf<String>() val current = StringBuilder() var inQuotes = false var quoteChar = ' ' for (char in line.trim()) { when { char == '"' || char == '\'' -> { if (!inQuotes) { inQuotes = true quoteChar = char } else if (char == quoteChar) { inQuotes = false } else { current.append(char) } } char.isWhitespace() && !inQuotes -> { if (current.isNotEmpty()) { parts.add(current.toString()) current.clear() } } else -> current.append(char) } } if (current.isNotEmpty()) parts.add(current.toString()) return ParsedCommand( name = parts.firstOrNull() ?: "", args = parts.drop(1) ) }
143-158: Path resolution doesn't normalize embedded.and..segments.
resolvePath("foo/../bar", "/home")returns/home/foo/../barinstead of/home/bar. Paths with embedded navigation segments won't resolve correctly.🔎 Suggested normalization:
internal fun resolvePath(path: String, workingDirectory: String): String { val absolutePath = when { path.startsWith("/") -> path else -> { val wd = if (workingDirectory.endsWith("/")) workingDirectory else "$workingDirectory/" wd + path } } // Normalize path segments val segments = absolutePath.split("/").filter { it.isNotEmpty() } val normalized = mutableListOf<String>() for (segment in segments) { when (segment) { "." -> { /* skip */ } ".." -> if (normalized.isNotEmpty()) normalized.removeLast() else -> normalized.add(segment) } } return "/" + normalized.joinToString("/") }xiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubCli.kt (1)
85-102: Consider propagating exit codes for CLI usage.When running commands via arguments, failures print errors but the process still exits with code 0. For proper CLI behavior, consider tracking and returning a non-zero exit code:
🔎 Suggested approach:
fun main(args: Array<String>): Unit = runBlocking { // ... setup code ... var exitCode = 0 if (args.isNotEmpty()) { for (command in args) { val result = shell.execute(command) // ... print output ... if (result.exitCode != 0) exitCode = result.exitCode } } else { runExamples(shell) } if (exitCode != 0) kotlin.system.exitProcess(exitCode) }xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/compose/FsRepository.kt (2)
23-24: Thread-safety concern with mutable caches.
dirCacheandfileCacheare plainmutableMapOfbut accessed from coroutines that may run on different threads. Consider usingConcurrentHashMapor synchronizing access with a mutex.🔎 Suggested fix using ConcurrentHashMap:
- private val dirCache = mutableMapOf<FsPath, MutableStateFlow<List<FsEntry>>>() - private val fileCache = mutableMapOf<FsPath, MutableStateFlow<ByteArray?>>() + private val dirCache = java.util.concurrent.ConcurrentHashMap<FsPath, MutableStateFlow<List<FsEntry>>>() + private val fileCache = java.util.concurrent.ConcurrentHashMap<FsPath, MutableStateFlow<ByteArray?>>()Note: For Kotlin Multiplatform, you may need a platform-agnostic solution like
kotlinx.atomicfuor aMutex-guarded wrapper.
69-73: Repeated calls create new StateFlows, risking memory leaks.Each call to
observeTextcreates a newStateFlowviastateIn(), even for the same path. In Compose, this could be called on every recomposition, leading to unbounded flow creation.🔎 Consider caching the text StateFlow:
+ private val textCache = mutableMapOf<FsPath, StateFlow<String>>() + fun observeText(path: String): StateFlow<String> { - return observeFile(path).map { bytes -> - bytes?.decodeToString() ?: "" - }.stateIn(scope, SharingStarted.Eagerly, "") + val fsPath = FsPath.of(path) + return textCache.getOrPut(fsPath) { + observeFile(path).map { bytes -> + bytes?.decodeToString() ?: "" + }.stateIn(scope, SharingStarted.Eagerly, "") + } }xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/BuiltinCommands.kt (1)
12-23: Consider usingargs.firstOrNull()for consistency.
lsusesargs.lastOrNull()while other commands useargs[0]orargs.firstOrNull(). For consistency and expected shell behavior, consider usingfirstOrNull().🔎 Suggested fix:
override suspend fun execute(args: List<String>, context: ShellContext): ShellResult { - val pathStr = resolvePath(args.lastOrNull() ?: ".", context.workingDirectory) + val pathStr = resolvePath(args.firstOrNull() ?: ".", context.workingDirectory)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreterTest.kt (1)
9-21: Consider adding edge case tests in a follow-up.The test coverage is solid. Consider adding tests for edge cases in a future iteration:
- Paths with special characters (spaces, quotes)
- Creating files/dirs at root level
- Deeply nested paths
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/XiuperFileSystem.kt (2)
72-78: Inconsistent policy check ordering betweendeleteand other write operations.In
delete,checkWritePolicyis called first, thenmount.policy.checkDeleteis called separately. However, other write operations (write,mkdir,commit) only callcheckWritePolicy. This inconsistency suggests either:
checkDeleteshould be integrated intocheckWritePolicy, or- Other operations may need similar granular policy hooks (e.g.,
checkMkdir).Consider whether the policy design is intentional or if
checkDeleteshould be handled uniformly.
115-155: Non-FsException errors are not audited.The
withAuditfunction only catchesFsException, meaning other exceptions (e.g.,IOException,CancellationException, network errors from backends) will propagate without being recorded in the audit log. This could leave gaps in observability.🔎 Consider catching all exceptions for audit completeness:
} catch (e: FsException) { val endTime = Clock.System.now() val latency = (endTime - startTime).inWholeMilliseconds auditCollector.collect( FsAuditEvent( operation = operation, path = path, backend = mount.backend::class.simpleName ?: "Unknown", status = FsOperationStatus.Failure(e.code, e.message ?: "Unknown error"), latencyMs = latency ) ) throw e + } catch (e: Exception) { + val endTime = Clock.System.now() + val latency = (endTime - startTime).inWholeMilliseconds + + auditCollector.collect( + FsAuditEvent( + operation = operation, + path = path, + backend = mount.backend::class.simpleName ?: "Unknown", + status = FsOperationStatus.Failure(FsErrorCode.EIO, e.message ?: "Unknown error"), + latencyMs = latency + ) + ) + + throw e }xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFs.kt (4)
23-27: HttpClient lifecycle concern: potential resource leak.The
HttpClientis created via factory but never closed. Ktor'sHttpClientholds resources (connection pools, coroutine scopes) that should be released when the backend is no longer used. Consider implementingCloseable/AutoCloseableor documenting ownership expectations.🔎 Consider adding close capability:
class RestFsBackend( private val service: RestServiceConfig, private val schema: RestSchema = RestSchema(), private val client: HttpClient = HttpClientFactory.create(service) -) : FsBackend, CapabilityAwareBackend { +) : FsBackend, CapabilityAwareBackend, AutoCloseable { + + override fun close() { + client.close() + }
131-137:HttpCalldata class has brokenequals/hashCodedue toByteArrayfield.
ByteArrayuses reference equality, so twoHttpCallinstances with identical content will not be equal. This can cause issues if used in collections or for comparison.🔎 Either override equals/hashCode or document the limitation:
data class HttpCall( val url: String, val method: HttpMethod, val headers: Map<String, String> = emptyMap(), val body: ByteArray? = null, val contentType: String? = null -) +) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is HttpCall) return false + return url == other.url && method == other.method && + headers == other.headers && body.contentEquals(other.body) && + contentType == other.contentType + } + + override fun hashCode(): Int { + var result = url.hashCode() + result = 31 * result + method.hashCode() + result = 31 * result + headers.hashCode() + result = 31 * result + (body?.contentHashCode() ?: 0) + result = 31 * result + (contentType?.hashCode() ?: 0) + return result + } +}
468-482:parseControlFilesilently ignores malformed lines.Lines with
=at position 0 (e.g.,=value) or blank keys after trimming are silently dropped. This is likely intentional for robustness, but consider logging warnings for malformed input to aid debugging.
265-267: Consider extracting the sharedJsonconfiguration.The
Jsoninstance withignoreUnknownKeys = true; isLenient = trueis created inline ingithubIssues(). If other schemas need similar configuration, this could lead to duplication. Consider extracting to a shared constant or factory.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/icons/AutoDevComposeIcons.kt(1 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt(5 hunks)mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt(1 hunks)settings.gradle.kts(1 hunks)xiuper-fs/README.md(1 hunks)xiuper-fs/build.gradle.kts(1 hunks)xiuper-fs/src/androidMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt(1 hunks)xiuper-fs/src/androidMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.android.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/BackendCapabilities.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsPath.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsTypes.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/XiuperFileSystem.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/compose/FsRepository.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/migrations/Migration.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/migrations/Migration_1_to_2.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFs.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFsAdvanced.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackend.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/FsAudit.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/MountPolicy.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/BuiltinCommands.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreter.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellTypes.kt(1 hunks)xiuper-fs/src/commonMain/sqldelight/cc/unitmesh/xiuper/fs/db/FsNode.sq(1 hunks)xiuper-fs/src/commonMain/sqldelight/cc/unitmesh/xiuper/fs/db/migrations/1.sqm(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/FsPathTest.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/XiuperVfsTest.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceRunner.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceTest.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackendTest.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/GitHubRestSchemaTest.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/RestSchemaBuilderTest.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackendTest.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/policy/PolicyTest.kt(1 hunks)xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreterTest.kt(1 hunks)xiuper-fs/src/iosMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt(1 hunks)xiuper-fs/src/iosMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.ios.kt(1 hunks)xiuper-fs/src/jsMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt(1 hunks)xiuper-fs/src/jsMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.js.kt(1 hunks)xiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt(1 hunks)xiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubCli.kt(1 hunks)xiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubShellExample.kt(1 hunks)xiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.jvm.kt(1 hunks)xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/conformance/DbPosixSubsetConformanceTest.kt(1 hunks)xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/conformance/RestFsConformanceTest.kt(1 hunks)xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationTest.kt(1 hunks)xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationUpgradeTest.kt(1 hunks)xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpFilesystemServerIntegrationTest.kt(1 hunks)xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/shell/DbFsShellSmokeTest.kt(1 hunks)xiuper-fs/src/wasmJsMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt(1 hunks)xiuper-fs/src/wasmJsMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.wasmJs.kt(1 hunks)xiuper-fs/xiuper-fs-db-migration-design.md(1 hunks)xiuper-ui/src/jvmTest/kotlin/cc/unitmesh/xuiper/render/HtmlRendererTest.kt(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualpattern for platform-specific code (for example:Platformimplementations)
Use concrete classes (not interfaces) andPromise(notFlow) for @JsExport annotations
Avoid emoji and UTF-8 characters in WASM code
When modifyingCodingAgentRenderer, update ALL implementations:DefaultCodingAgentRenderer,ComposeRenderer,JewelRenderer,ServerSideRenderer, andJsRendererAdapterin Kotlin
When modifyingCodingAgentRenderer, update JVM CLI implementations:CodingCliRendererandConsoleRenderer
UseAutoDevColorsorMaterialTheme.colorSchemefor Compose styling
Files:
xiuper-ui/src/jvmTest/kotlin/cc/unitmesh/xuiper/render/HtmlRendererTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/BackendCapabilities.ktxiuper-fs/src/iosMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/policy/PolicyTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellTypes.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.ktxiuper-fs/src/wasmJsMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.jvm.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsPath.ktxiuper-fs/src/androidMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.android.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/FsPathTest.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/icons/AutoDevComposeIcons.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/migrations/Migration.ktxiuper-fs/src/iosMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.ios.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackendTest.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/XiuperVfsTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackendTest.ktxiuper-fs/src/androidMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubCli.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsTypes.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceRunner.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/RestSchemaBuilderTest.ktxiuper-fs/src/jsMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.js.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreter.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationTest.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpFilesystemServerIntegrationTest.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/conformance/RestFsConformanceTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/migrations/Migration_1_to_2.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/FsAudit.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFsAdvanced.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationUpgradeTest.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/shell/DbFsShellSmokeTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackend.ktxiuper-fs/src/wasmJsMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.wasmJs.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/conformance/DbPosixSubsetConformanceTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/MountPolicy.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.ktxiuper-fs/src/jsMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/XiuperFileSystem.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/compose/FsRepository.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubShellExample.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/BuiltinCommands.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreterTest.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFs.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/GitHubRestSchemaTest.kt
**/*.{kt,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never use hardcoded colors - always use design tokens from the design system
Files:
xiuper-ui/src/jvmTest/kotlin/cc/unitmesh/xuiper/render/HtmlRendererTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/BackendCapabilities.ktxiuper-fs/src/iosMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/policy/PolicyTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellTypes.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.ktxiuper-fs/src/wasmJsMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.jvm.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsPath.ktxiuper-fs/src/androidMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.android.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/FsPathTest.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/icons/AutoDevComposeIcons.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/migrations/Migration.ktxiuper-fs/src/iosMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.ios.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackendTest.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/XiuperVfsTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackendTest.ktxiuper-fs/src/androidMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubCli.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsTypes.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceRunner.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/RestSchemaBuilderTest.ktxiuper-fs/src/jsMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.js.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreter.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationTest.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpFilesystemServerIntegrationTest.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/conformance/RestFsConformanceTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/migrations/Migration_1_to_2.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/FsAudit.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFsAdvanced.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationUpgradeTest.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/shell/DbFsShellSmokeTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackend.ktxiuper-fs/src/wasmJsMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.wasmJs.ktxiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/conformance/DbPosixSubsetConformanceTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/MountPolicy.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.ktxiuper-fs/src/jsMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/XiuperFileSystem.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/compose/FsRepository.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubShellExample.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/BuiltinCommands.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreterTest.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceTest.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFs.ktxiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/GitHubRestSchemaTest.kt
🧠 Learnings (4)
📚 Learning: 2025-12-07T08:40:10.859Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.859Z
Learning: Applies to **/*.kt : When modifying `CodingAgentRenderer`, update ALL implementations: `DefaultCodingAgentRenderer`, `ComposeRenderer`, `JewelRenderer`, `ServerSideRenderer`, and `JsRendererAdapter` in Kotlin
Applied to files:
xiuper-ui/src/jvmTest/kotlin/cc/unitmesh/xuiper/render/HtmlRendererTest.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt
📚 Learning: 2025-12-07T08:40:10.859Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.859Z
Learning: Applies to **/*.kt : When modifying `CodingAgentRenderer`, update JVM CLI implementations: `CodingCliRenderer` and `ConsoleRenderer`
Applied to files:
xiuper-ui/src/jvmTest/kotlin/cc/unitmesh/xuiper/render/HtmlRendererTest.ktxiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/github/examples/GitHubCli.kt
📚 Learning: 2025-12-07T08:40:10.859Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.859Z
Learning: Applies to **/*.kt : Use `AutoDevColors` or `MaterialTheme.colorScheme` for Compose styling
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt
📚 Learning: 2025-12-07T08:40:10.859Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.859Z
Learning: Always consider all platforms when working with Kotlin Multiplatform: JS, WASM, Desktop JVM, Android, iOS
Applied to files:
xiuper-fs/build.gradle.kts
🧬 Code graph analysis (13)
xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackendTest.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/test/DevInsTestCase.kt (1)
assertTrue(29-33)
xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/XiuperVfsTest.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/test/DevInsTestCase.kt (1)
assertEquals(45-49)
xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceRunner.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/test/DevInsTestCase.kt (2)
assertTrue(29-33)assertEquals(45-49)
xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/http/RestSchemaBuilderTest.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/test/DevInsTestCase.kt (2)
assertNotNull(54-59)assertEquals(45-49)
xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/mcp/McpFilesystemServerIntegrationTest.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/test/DevInsTestCase.kt (2)
assertTrue(29-33)assertNotNull(54-59)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFsAdvanced.kt (1)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFs.kt (8)
stat(36-38)stat(193-199)stat(231-234)dir(236-239)read(44-61)read(241-244)write(63-85)write(246-249)
xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/shell/DbFsShellSmokeTest.kt (1)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.kt (1)
DbFsBackend(148-158)
xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/conformance/DbPosixSubsetConformanceTest.kt (1)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.kt (1)
DbFsBackend(148-158)
xiuper-fs/build.gradle.kts (6)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFs.kt (1)
create(485-485)xiuper-fs/src/androidMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.android.kt (1)
create(14-33)xiuper-fs/src/iosMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.ios.kt (1)
create(14-33)xiuper-fs/src/jsMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.js.kt (1)
create(14-33)xiuper-fs/src/jvmMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.jvm.kt (1)
create(14-33)xiuper-fs/src/wasmJsMain/kotlin/cc/unitmesh/xiuper/fs/http/HttpClientFactory.wasmJs.kt (1)
create(14-33)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/XiuperFileSystem.kt (1)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsPath.kt (1)
resolve(18-22)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/BuiltinCommands.kt (1)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreter.kt (1)
resolvePath(143-159)
xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/shell/ShellFsInterpreterTest.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/test/DevInsTestCase.kt (2)
assertEquals(45-49)assertTrue(29-33)
xiuper-fs/src/commonTest/kotlin/cc/unitmesh/xiuper/fs/conformance/PosixSubsetConformanceTest.kt (1)
xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/conformance/DbPosixSubsetConformanceTest.kt (2)
newTarget(22-42)assertFsError(210-218)
🪛 LanguageTool
xiuper-fs/README.md
[uncategorized] ~200-~200: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...al workflows (DeleteApprovalPolicy) - Rate limiting hooks ## Related Documentation - Desi...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Build
- GitHub Check: Test mpp-core (KMP)
| ### Phase 4: Advanced Policies 📋 | ||
| - Path allowlist/denylist (`PathFilterPolicy`) | ||
| - Delete approval workflows (`DeleteApprovalPolicy`) | ||
| - Rate limiting hooks |
There was a problem hiding this comment.
Minor: Use hyphen in compound adjective.
Change "Rate limiting hooks" to "Rate-limiting hooks" for grammatical correctness when the compound adjective modifies a noun.
✏️ Apply this correction:
-- Rate limiting hooks
+- Rate-limiting hooks📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Rate limiting hooks | |
| - Rate-limiting hooks |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~200-~200: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...al workflows (DeleteApprovalPolicy) - Rate limiting hooks ## Related Documentation - Desi...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🤖 Prompt for AI Agents
In xiuper-fs/README.md around line 200, the phrase "Rate limiting hooks" should
use a hyphen as a compound adjective; change it to "Rate-limiting hooks" to
correct grammar where the compound modifies a noun.
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.kt
Show resolved
Hide resolved
| override suspend fun stat(path: FsPath): FsStat { | ||
| val parsed = parsePath(path.value) | ||
|
|
||
| return try { | ||
| val response: HttpResponse = client.get("$baseUrl/repos/${parsed.owner}/${parsed.repo}/contents/${parsed.path}") { | ||
| parameter("ref", parsed.ref) | ||
| token?.let { header("Authorization", "Bearer $it") } | ||
| } | ||
|
|
||
| if (!response.status.isSuccess()) { | ||
| throw FsException(FsErrorCode.ENOENT, "Path not found: ${path.value}") | ||
| } | ||
|
|
||
| val content: GitHubContent = json.decodeFromString(response.bodyAsText()) | ||
|
|
||
| FsStat( | ||
| path = path, | ||
| isDirectory = content.type == "dir", | ||
| size = content.size?.toLong() | ||
| ) | ||
| } catch (e: FsException) { | ||
| throw e | ||
| } catch (e: Exception) { | ||
| throw FsException(FsErrorCode.EIO, "Failed to stat ${path.value}: ${e.message}") | ||
| } | ||
| } |
There was a problem hiding this comment.
JSON parsing will fail for directory paths.
When path points to a directory, GitHub's Contents API returns a JSON array, not a single object. Line 61 attempts to deserialize as GitHubContent, which will throw a SerializationException for directories.
🔎 Suggested fix - try parsing as array first:
override suspend fun stat(path: FsPath): FsStat {
val parsed = parsePath(path.value)
return try {
val response: HttpResponse = client.get("$baseUrl/repos/${parsed.owner}/${parsed.repo}/contents/${parsed.path}") {
parameter("ref", parsed.ref)
token?.let { header("Authorization", "Bearer $it") }
}
if (!response.status.isSuccess()) {
throw FsException(FsErrorCode.ENOENT, "Path not found: ${path.value}")
}
- val content: GitHubContent = json.decodeFromString(response.bodyAsText())
-
- FsStat(
- path = path,
- isDirectory = content.type == "dir",
- size = content.size?.toLong()
- )
+ val body = response.bodyAsText()
+ // Directory returns array, file returns object
+ val isDirectory = body.trimStart().startsWith("[")
+ if (isDirectory) {
+ FsStat(path = path, isDirectory = true, size = null)
+ } else {
+ val content: GitHubContent = json.decodeFromString(body)
+ FsStat(
+ path = path,
+ isDirectory = content.type == "dir",
+ size = content.size?.toLong()
+ )
+ }🤖 Prompt for AI Agents
In
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.kt
around lines 48 to 73, the code always deserializes the response as a single
GitHubContent but GitHub returns a JSON array for directory paths; update the
parsing to handle both shapes: first read the response body as text, if it
represents a JSON array (e.g., starts with '[' or by attempting to decode a
List<GitHubContent>), treat it as a directory and return FsStat(path=path,
isDirectory=true, size=null or 0L), otherwise decode to a single GitHubContent
and return the file stat; keep existing error handling and map non-success HTTP
statuses to ENOENT as before.
| internal class RestRuntimeState : RestSchemaRuntime { | ||
| private val queryByScope: MutableMap<String, Map<String, String>> = mutableMapOf() | ||
| override fun setQuery(scopePath: String, query: Map<String, String>) { | ||
| queryByScope[scopePath] = query | ||
| } | ||
|
|
||
| override fun getQuery(scopePath: String): Map<String, String>? = queryByScope[scopePath] | ||
| } |
There was a problem hiding this comment.
RestRuntimeState is not thread-safe.
The queryByScope map is a plain MutableMap with no synchronization. If RestFsBackend is used concurrently (which is likely given suspend functions), concurrent reads/writes to queryByScope could cause data races.
🔎 Consider using a concurrent map:
+import java.util.concurrent.ConcurrentHashMap
+
internal class RestRuntimeState : RestSchemaRuntime {
- private val queryByScope: MutableMap<String, Map<String, String>> = mutableMapOf()
+ private val queryByScope: MutableMap<String, Map<String, String>> = ConcurrentHashMap()
override fun setQuery(scopePath: String, query: Map<String, String>) {
queryByScope[scopePath] = query
}Note: For Kotlin Multiplatform, you may need kotlinx.atomicfu or platform-specific solutions since ConcurrentHashMap is JVM-only.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/http/RestFs.kt around
lines 157 to 164, queryByScope is a plain MutableMap and not thread-safe; wrap
all accesses to queryByScope with synchronization to prevent data races (e.g.,
use synchronized(this) in setQuery and getQuery), or replace the map with a
platform-appropriate concurrent implementation (ConcurrentHashMap on JVM via an
expect/actual or use kotlinx.atomicfu/kotlinx.coroutines Mutex for
multiplatform) and ensure both setter and getter use that thread-safe mechanism.
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.kt
Show resolved
Hide resolved
| data class Wildcard(val pattern: String) : PathPattern { | ||
| private val regex = pattern | ||
| .replace(".", "\\.") | ||
| .replace("*", ".*") | ||
| .toRegex() | ||
|
|
||
| override fun matches(path: FsPath): Boolean = regex.matches(path.value) | ||
| } |
There was a problem hiding this comment.
Potential ReDoS vulnerability with complex wildcard patterns.
The regex is created from user-provided patterns with .* substitution. Malicious patterns like ***** could cause catastrophic backtracking. Consider limiting pattern complexity or using a simpler glob matcher.
🔎 Suggested mitigation:
data class Wildcard(val pattern: String) : PathPattern {
- private val regex = pattern
- .replace(".", "\\.")
- .replace("*", ".*")
- .toRegex()
+ private val regex: Regex
+
+ init {
+ require(pattern.length <= 256) { "Pattern too long" }
+ require(pattern.count { it == '*' } <= 10) { "Too many wildcards" }
+ regex = pattern
+ .replace(".", "\\.")
+ .replace("*", "[^/]*") // Non-greedy, single segment match
+ .toRegex()
+ }Using [^/]* instead of .* is also more semantically correct for path segments.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data class Wildcard(val pattern: String) : PathPattern { | |
| private val regex = pattern | |
| .replace(".", "\\.") | |
| .replace("*", ".*") | |
| .toRegex() | |
| override fun matches(path: FsPath): Boolean = regex.matches(path.value) | |
| } | |
| data class Wildcard(val pattern: String) : PathPattern { | |
| private val regex: Regex | |
| init { | |
| require(pattern.length <= 256) { "Pattern too long" } | |
| require(pattern.count { it == '*' } <= 10) { "Too many wildcards" } | |
| regex = pattern | |
| .replace(".", "\\.") | |
| .replace("*", "[^/]*") // Non-greedy, single segment match | |
| .toRegex() | |
| } | |
| override fun matches(path: FsPath): Boolean = regex.matches(path.value) | |
| } |
🤖 Prompt for AI Agents
In xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/MountPolicy.kt
around lines 73 to 80, the current pattern -> regex conversion uses ".*" for "*"
which can lead to catastrophic backtracking and ReDoS on malicious inputs;
change the transformation to (1) escape regex metacharacters (e.g., replace "."
with "\\."), (2) collapse consecutive "*" into a single wildcard (e.g.,
replace(Regex("\\*+"), "*")), (3) replace each "*" with a non-greedy segment
matcher "[^/]*" (so wildcards don't cross path separators), (4) anchor the
pattern with ^ and $ and compile as Regex; additionally enforce a sane limit
(max length or max wildcard count) before compiling to reject overly complex
patterns. This replaces ".*" with "[^/]*", prevents excessive wildcard
repetition, and bounds pattern complexity to mitigate ReDoS.
| class MvCommand : ShellCommand { | ||
| override val name = "mv" | ||
| override val description = "Move/rename file" | ||
|
|
||
| override suspend fun execute(args: List<String>, context: ShellContext): ShellResult { | ||
| if (args.size < 2) { | ||
| return ShellResult(1, "", "mv: missing file operand") | ||
| } | ||
|
|
||
| val sourceStr = resolvePath(args[0], context.workingDirectory) | ||
| val destStr = resolvePath(args[1], context.workingDirectory) | ||
|
|
||
| return try { | ||
| val result = context.backend.read(FsPath.of(sourceStr), ReadOptions()) | ||
| context.backend.write(FsPath.of(destStr), result.bytes, WriteOptions()) | ||
| context.backend.delete(FsPath.of(sourceStr)) | ||
| ShellResult(0, "", "") | ||
| } catch (e: Exception) { | ||
| ShellResult(1, "", "mv: cannot move '$sourceStr' to '$destStr': ${e.message}") | ||
| } | ||
| } |
There was a problem hiding this comment.
Data integrity risk: partial failure leaves duplicate files.
If delete fails after write succeeds (line 198-199), the file exists in both source and destination. Consider catching errors from each step separately or at least noting the limitation.
🔎 Suggested improvement with better error handling:
return try {
val result = context.backend.read(FsPath.of(sourceStr), ReadOptions())
context.backend.write(FsPath.of(destStr), result.bytes, WriteOptions())
- context.backend.delete(FsPath.of(sourceStr))
- ShellResult(0, "", "")
+ try {
+ context.backend.delete(FsPath.of(sourceStr))
+ ShellResult(0, "", "")
+ } catch (e: Exception) {
+ // Write succeeded but delete failed - file is duplicated
+ ShellResult(1, "", "mv: copied to '$destStr' but failed to remove '$sourceStr': ${e.message}")
+ }
} catch (e: Exception) {
ShellResult(1, "", "mv: cannot move '$sourceStr' to '$destStr': ${e.message}")
}🤖 Prompt for AI Agents
In
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/shell/BuiltinCommands.kt
around lines 183 to 203, the current mv implementation performs write then
delete inside a single try/catch which can leave both source and destination
present if delete fails; change to handle each step separately: first read
source, then write destination, then attempt to delete source in its own
try/catch; if delete fails, attempt to roll back by deleting the newly written
destination (and surface a clear error that includes both operations and any
rollback result); ensure exceptions from read/write/delete are caught
individually and return appropriate ShellResult messages and non-zero codes.
| private fun resolve(path: FsPath): Pair<Mount, FsPath> { | ||
| val normalized = FsPath.of(path.value) | ||
| val mount = normalizedMounts.firstOrNull { isUnderMount(normalized, it.mountPoint) } | ||
| ?: throw FsException(FsErrorCode.ENOENT, "No mount for path: ${normalized.value}") | ||
|
|
||
| val inner = stripMount(normalized, mount.mountPoint) | ||
| return mount to inner | ||
| } |
There was a problem hiding this comment.
Mount resolution exception bypasses auditing.
The resolve(path) call at line 120 happens before the try block's operation, so if no mount is found, the FsException is thrown without being recorded in the audit log. This is a gap in observability for "mount not found" errors.
Consider restructuring to audit resolution failures, or document this as intentional behavior.
🤖 Prompt for AI Agents
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/XiuperFileSystem.kt around
lines 94 to 101: the call to resolve(path) currently lives outside the
operation's try block so FsException for "No mount for path" bypasses the audit;
wrap the mount resolution in the same try/catch that audits the operation (or
perform a guarded resolve: try resolving inside a small try that calls the audit
logging function with the FsException and rethrows), ensuring that when resolve
fails you call the existing audit recording with the error details before
rethrowing so "mount not found" is recorded consistently with other filesystem
errors.
| private fun setUserVersion(driver: SqlDriver, version: Int) { | ||
| driver.execute( | ||
| identifier = null, | ||
| sql = "PRAGMA user_version = $version", | ||
| parameters = 0, | ||
| binders = null | ||
| ) | ||
| } |
There was a problem hiding this comment.
SQL injection risk in setUserVersion.
Line 203 uses string interpolation ("PRAGMA user_version = $version") which could be exploited if version is ever derived from untrusted input. While currently an Int, parameterization is safer.
🔎 Suggested fix:
Note that SQLite's PRAGMA statements don't support parameterized queries, so the safest approach is to validate the version is a positive integer:
private fun setUserVersion(driver: SqlDriver, version: Int) {
require(version >= 0) { "Version must be non-negative" }
driver.execute(
identifier = null,
sql = "PRAGMA user_version = $version",
parameters = 0,
binders = null
)
}🤖 Prompt for AI Agents
In xiuper-fs/xiuper-fs-db-migration-design.md around lines 200 to 207, the
method builds the PRAGMA SQL via string interpolation which is unsafe if version
ever came from untrusted input; since SQLite PRAGMA cannot be parameterized,
validate the input first by requiring version to be a non-negative integer
(e.g., require(version >= 0) with an explanatory message) before calling
driver.execute, so the SQL string can safely interpolate the validated value;
keep the PRAGMA call as-is after adding this guard.
- Fix DbFsBackend delete() to handle multiple children correctly - Fix McpBackend stat() URI encoding/decoding mismatch - Add AutoCloseable to GitHubFsBackend for proper resource management - Replace hardcoded colors with MaterialTheme.colorScheme in NanoDSLBlockRenderer - Implement proper equals/hashCode for ReadResult ByteArray - Add defensive copies in InMemoryFsBackend read/write - Fix PathPattern.Wildcard ReDoS vulnerability with pattern limits - Add documentation for SQLDelight Schema.version auto-generation
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt (1)
318-373: Remove the unused NanoDSLToggleButton component.The
NanoDSLToggleButtonis not used anywhere in the codebase and has been replaced byNanoDSLIconButtonin this file. Delete it to reduce maintenance overhead.
♻️ Duplicate comments (3)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackend.kt (2)
198-222: Decode base64 blob content to binary bytes.Line 211 retrieves
BlobResourceContents.blob, which is a base64-encoded string per the MCP specification. However, line 216 calls.encodeToByteArray()on it, which converts the base64 string characters to UTF-8 bytes instead of decoding the base64 content to binary. This will corrupt binary resources like images or PDFs.Since this is multiplatform Kotlin code, add an
expect/actualbase64 decoder or use a multiplatform library. For JVM, usejava.util.Base64.getDecoder().decode(firstContent.blob).🔎 Suggested approach for multiplatform base64 decoding:
Option 1: Add expect/actual platform utilities:
// In commonMain expect fun decodeBase64(encoded: String): ByteArray // In jvmMain actual fun decodeBase64(encoded: String): ByteArray { return java.util.Base64.getDecoder().decode(encoded) }Then update line 209-216:
val text = when (firstContent) { is io.modelcontextprotocol.kotlin.sdk.TextResourceContents -> firstContent.text - is io.modelcontextprotocol.kotlin.sdk.BlobResourceContents -> firstContent.blob + is io.modelcontextprotocol.kotlin.sdk.BlobResourceContents -> { + return ReadResult( + bytes = decodeBase64(firstContent.blob), + contentType = firstContent.mimeType + ) + } else -> throw FsException(FsErrorCode.EIO, "Unknown content type") } return ReadResult( bytes = text.encodeToByteArray(), contentType = firstContent.mimeType )Option 2: Use a multiplatform base64 library like
kotlinx-ioorokioif already in dependencies.
248-254: URI encoding collision risk remains unresolved.The simple "/" → "_" replacement creates collisions where distinct MCP resource URIs map to the same filename:
- URI
"dir/file"→ encoded as"dir_file"- URI
"dir_file"→ encoded as"dir_file"Both URIs become indistinguishable, making one resource inaccessible.
Use collision-free encoding such as URL percent-encoding or base64. For Kotlin multiplatform, consider:
🔎 Suggested fixes:
Option 1: URL encoding with Ktor (if available):
+import io.ktor.http.encodeURLPath +import io.ktor.http.decodeURLQueryComponent + private fun encodeUri(uri: String): String { - return uri.replace("/", "_") + return uri.encodeURLPath() } private fun decodeUri(encoded: String): String { - return encoded.replace("_", "/") + return encoded.decodeURLQueryComponent() }Option 2: Base64 URL-safe encoding (multiplatform with expect/actual):
// In commonMain expect fun encodeBase64Url(text: String): String expect fun decodeBase64Url(encoded: String): String // In jvmMain actual fun encodeBase64Url(text: String): String { return java.util.Base64.getUrlEncoder().withoutPadding() .encodeToString(text.encodeToByteArray()) } actual fun decodeBase64Url(encoded: String): String { return java.util.Base64.getUrlDecoder() .decode(encoded).decodeToString() } // Then use these in encodeUri/decodeUrixiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.kt (1)
56-81: Critical: stat() will crash on directory paths.Line 69 attempts to deserialize the response as a single
GitHubContentobject, but the GitHub Contents API returns a JSON array for directory paths. This will throw aSerializationExceptionwhen callingstat()on any directory.🔎 Suggested fix:
override suspend fun stat(path: FsPath): FsStat { val parsed = parsePath(path.value) return try { val response: HttpResponse = client.get("$baseUrl/repos/${parsed.owner}/${parsed.repo}/contents/${parsed.path}") { parameter("ref", parsed.ref) token?.let { header("Authorization", "Bearer $it") } } if (!response.status.isSuccess()) { throw FsException(FsErrorCode.ENOENT, "Path not found: ${path.value}") } - val content: GitHubContent = json.decodeFromString(response.bodyAsText()) - - FsStat( - path = path, - isDirectory = content.type == "dir", - size = content.size?.toLong() - ) + val body = response.bodyAsText() + // GitHub returns array for directories, object for files + val isDirectory = body.trimStart().startsWith("[") + if (isDirectory) { + FsStat(path = path, isDirectory = true, size = null) + } else { + val content: GitHubContent = json.decodeFromString(body) + FsStat( + path = path, + isDirectory = content.type == "dir", + size = content.size?.toLong() + ) + } } catch (e: FsException) { throw e } catch (e: Exception) { throw FsException(FsErrorCode.EIO, "Failed to stat ${path.value}: ${e.message}") } }
🧹 Nitpick comments (1)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.kt (1)
28-60: Optional: Consider query optimization for deep hierarchies.The current implementation uses a
LIKEpattern to fetch all descendants, then filters in-memory to derive immediate children. For deep directory trees, this may scan many rows unnecessarily.This approach works correctly for moderate hierarchies. If performance becomes a concern, consider:
- Adding a
parent_pathcolumn to enable direct parent-child queries- Using SQLite's JSON or recursive CTEs for path hierarchies
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt(6 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsTypes.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackend.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.kt(1 hunks)xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/MountPolicy.kt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualpattern for platform-specific code (for example:Platformimplementations)
Use concrete classes (not interfaces) andPromise(notFlow) for @JsExport annotations
Avoid emoji and UTF-8 characters in WASM code
When modifyingCodingAgentRenderer, update ALL implementations:DefaultCodingAgentRenderer,ComposeRenderer,JewelRenderer,ServerSideRenderer, andJsRendererAdapterin Kotlin
When modifyingCodingAgentRenderer, update JVM CLI implementations:CodingCliRendererandConsoleRenderer
UseAutoDevColorsorMaterialTheme.colorSchemefor Compose styling
Files:
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsTypes.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/MountPolicy.kt
**/*.{kt,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never use hardcoded colors - always use design tokens from the design system
Files:
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsTypes.ktmpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/github/GitHubFsBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.ktxiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/MountPolicy.kt
🧠 Learnings (2)
📚 Learning: 2025-12-07T08:40:10.859Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.859Z
Learning: Applies to **/*.kt : When modifying `CodingAgentRenderer`, update ALL implementations: `DefaultCodingAgentRenderer`, `ComposeRenderer`, `JewelRenderer`, `ServerSideRenderer`, and `JsRendererAdapter` in Kotlin
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt
📚 Learning: 2025-12-07T08:40:10.859Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-07T08:40:10.859Z
Learning: Applies to **/*.kt : Use `AutoDevColors` or `MaterialTheme.colorScheme` for Compose styling
Applied to files:
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt
🧬 Code graph analysis (1)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt (1)
xiuper-fs/src/jvmTest/kotlin/cc/unitmesh/xiuper/fs/db/migrations/MigrationTest.kt (2)
getUserVersion(84-98)setUserVersion(100-107)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test mpp-core (KMP)
- GitHub Check: Build
🔇 Additional comments (16)
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/mcp/McpBackend.kt (7)
9-31: LGTM! Clear interface design.The interface provides a clean abstraction for MCP-backed filesystem operations with well-documented directory structure mapping.
38-43: LGTM! Reasonable caching strategy.The lazy-loaded resource and tool caches minimize redundant MCP calls, and the args cache enables the write-to-execute pattern for tools.
50-61: Past concern resolved: URI decoding is correct.The code properly decodes the URI at line 53 before calling
getResource(uri)at line 54, which is consistent with the encoding applied inlist()at line 88. The past review comment concern has been addressed.
79-114: LGTM! Consistent directory structure implementation.The listing logic correctly implements the documented directory structure and consistently encodes resource URIs.
116-139: LGTM! Read operations correctly handle path types.The read logic properly decodes resource URIs and appropriately restricts tool "run" files to write-only access.
141-169: LGTM! Effective write-to-execute pattern for tools.The implementation correctly supports writing JSON arguments and triggering tool execution via the "run" file, with proper validation and error handling.
171-177: LGTM! Appropriate unsupported operations.Throwing
ENOTSUPfor delete and mkdir is correct since MCP protocol doesn't support filesystem mutations on the remote side.mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/sketch/NanoDSLBlockRenderer.kt (4)
241-252: LGTM! Proper theme-aware color implementation.The replacement of hardcoded color values with
MaterialTheme.colorSchemefollows the coding guidelines and uses semantically appropriate color tokens (surface variants for backgrounds, outline variants for borders).
267-274: LGTM! Clean zoom implementation.The zoom transformation is correctly applied via
Modifier.scale(zoomLevel)within the themed context, ensuring the rendered content scales while preserving theme colors.
375-446: LGTM! Well-designed icon button component.The
NanoDSLIconButtoncomponent is properly implemented with theme-aware colors, smooth animations, and accessibility support. The icon-based approach is more robust for cross-platform deployment (especially WASM) compared to emoji-based controls.
70-70: Icon verification passed—all 9 referenced icons exist in AutoDevComposeIcons.kt.The zoom controls, theme-aware buttons, and icon-based implementation are properly implemented. Zoom boundaries are enforced (0.5f–2.0f with 0.1f increments), colors use
MaterialTheme.colorSchemeper guidelines, and the Material icon approach avoids emoji/UTF-8 issues for WASM compatibility. All icons are sourced from the standard Material Design library.xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DbFsBackend.kt (1)
110-128: LGTM - past issue resolved.The delete method now correctly uses
executeAsList().firstOrNull()to check for children, avoiding the multi-row exception that could occur withexecuteAsOneOrNull(). The ENOTEMPTY enforcement is now safe and correct.xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/FsTypes.kt (1)
71-87: LGTM - ByteArray equality properly implemented.The
ReadResultdata class now correctly overridesequals()andhashCode()to use content-based comparison for theByteArrayfield. This resolves the past issue where two instances with identical byte content wouldn't be considered equal.xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/memory/InMemoryFsBackend.kt (1)
47-76: LGTM - defensive copying properly implemented.The
read()andwrite()methods now use defensive copying (copyOf()) to prevent external code from mutating the backend's internal state. This ensures the in-memory filesystem remains consistent and isolated.xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/policy/MountPolicy.kt (1)
73-89: LGTM - ReDoS mitigations properly implemented.The
Wildcardpattern implementation now includes proper protections against ReDoS attacks:
- Pattern length limited to 256 characters
- Maximum of 10 wildcards allowed
- Uses
[^/]*for segment-local matching (prevents greedy cross-segment matches)This effectively mitigates the catastrophic backtracking concern from the past review.
xiuper-fs/src/commonMain/kotlin/cc/unitmesh/xiuper/fs/db/DatabaseDriverFactory.kt (1)
19-57: Schema version aligns correctly with MigrationRegistry.The migration setup is verified:
- 1 .sqm file exists, resulting in Schema.version = 2 (base + 1 migration) ✓
- MigrationRegistry.path(currentVersion, targetVersion) correctly produces complete migration paths with proper validation ✓
- MigrationRegistry.all contains Migration_1_to_2() matching the available migrations ✓
No alignment issues found.
✅ All Review Comments AddressedI've fixed all the review comments from the three bot reviewers: 🔧 Augment Review (3 critical issues)
🔧 Copilot Review (2 issues)
🔧 CodeRabbit Review (critical issues)
✅ Test ResultsAll tests pass successfully:
Note: iOS build fails due to MCP SDK not supporting iOS platform - this is a pre-existing limitation unrelated to these changes. The PR is now ready for review and acceptance! 🚀 |
|
Sorry, Augment failed to review this pull request. |
1 participant
Summary by CodeRabbit
New Features
UI Improvements
Docs & Examples
✏️ Tip: You can customize this high-level summary in your review settings.