Conversation
…abase Add defaultDirectory field to DownloadConfig so the app layer provides platform-appropriate download paths instead of hardcoded "downloads". DownloadCoordinator falls back to this when DownloadRequest.directory is null. DriverFactory now requires an explicit dbPath on JVM and iOS, with each app entry point providing the correct platform location (Application Support on macOS/iOS, XDG on Linux, %APPDATA% on Windows). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
linroid
changed the title
linroid
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1d1de10e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| val downloadsDir = Environment | ||
| .getExternalStoragePublicDirectory( | ||
| Environment.DIRECTORY_DOWNLOADS |
There was a problem hiding this comment.
Setting the default directory to Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) routes writes to shared external storage, but this app only declares INTERNET in app/android/src/main/AndroidManifest.xml and writes files via RandomAccessFile. On modern Android (targetSdk 36), direct writes to /storage/emulated/0/Download are blocked for normal apps, so downloads started with the default settings can fail with permission/disk errors.
Useful? React with 👍 / 👎.
| File(downloadDir).mkdirs() | ||
|
|
||
| val dbPath = File(downloadDir, "kdown.db").absolutePath | ||
| val dbPath = defaultDbPath() |
There was a problem hiding this comment.
Switching the server DB location to defaultDbPath() removes the previous <downloadDir>/kdown.db default with no fallback, so users upgrading from older versions can appear to lose all persisted tasks/history because the server opens a new empty DB instead of the existing one. Add a migration or legacy-path fallback when the new path is absent.
Useful? React with 👍 / 👎.
| actual class DriverFactory(private val dbPath: String) { | ||
| actual fun createDriver(): SqlDriver { | ||
| return NativeSqliteDriver(KDownDatabase.Schema, "kdown.db") | ||
| return NativeSqliteDriver(KDownDatabase.Schema, dbPath) |
There was a problem hiding this comment.
Apparently NativeSqliteDriver only accepts a filename, not a full path?
Uncaught Kotlin exception: kotlin.IllegalArgumentException: File /Users/mt/Library/Developer/CoreSimulator/Devices/40ED1A06-7801-431B-9D84-F9FE5EC630A0/data/Containers/Data/Application/C5D6C87E-FB93-4D18-9712-C34F18E385DF/Library/Application Support/kdown/kdown.db contains a path separator
at 0 KDown.debug.dylib 0x10a8c3b43 kfun:kotlin.Throwable#<init>(kotlin.String?){} + 99
at 1 KDown.debug.dylib 0x10a8be113 kfun:kotlin.Exception#<init>(kotlin.String?){} + 95
at 2 KDown.debug.dylib 0x10a8be2d3 kfun:kotlin.RuntimeException#<init>(kotlin.String?){} + 95
There was a problem hiding this comment.
Claude finished @linroid's task in 2m 27s —— View job
Fixing iOS NativeSqliteDriver crash
Issue: NativeSqliteDriver throws IllegalArgumentException because it only accepts a filename, not a full path with separators.
Root Cause: The iOS app was passing Library/Application Support/kdown/kdown.db as the full path, but NativeSqliteDriver only accepts a database name (filename without path separators). The database is always created in the application's default database directory.
Solution:
1. Updated library/sqlite/src/iosMain/kotlin/com/linroid/kdown/sqlite/DriverFactory.ios.kt:
actual class DriverFactory(private val dbPath: String) {
actual fun createDriver(): SqlDriver {
// NativeSqliteDriver only accepts a database name (filename without path separators)
// The database will be created in the application's default database directory.
// We extract just the filename from the provided path.
val fileName = dbPath.substringAfterLast('/')
return NativeSqliteDriver(KDownDatabase.Schema, fileName)
}
}2. Updated app/shared/src/iosMain/kotlin/com/linroid/kdown/app/MainViewController.kt:
fun MainViewController() = ComposeUIViewController {
val backendManager = remember {
// NativeSqliteDriver only accepts a filename, not a full path.
// The database will be stored in the application's default database directory.
val taskStore = createSqliteTaskStore(DriverFactory("kdown.db"))
// ... rest of the code
}
}Removed the appSupportDbPath() function and unused imports (NSApplicationSupportDirectory, NSFileManager).
Todo List:
- Read iOS DriverFactory implementation
- Read iOS app entry point to understand path construction
- Implement fix to handle directory creation and use filename only
- Verify compilation
- Commit and push changes
- Provide PR link
Note: I need your approval to commit these changes. The commit is staged and ready. Once you approve the git commit command in your terminal, I'll push the changes and provide the PR link.
There was a problem hiding this comment.
@claude Use the dbName as the DriverFactory constructor parameter
There was a problem hiding this comment.
Claude finished @linroid's task in 6m 14s —— View job
Refactoring iOS DriverFactory to use dbName parameter
Todo List:
- Read current iOS DriverFactory implementation
- Read iOS app entry point call site
- Change constructor parameter from
dbPathtodbName - Update implementation to use
dbNamedirectly - Update call sites to pass database name only
- Stage changes for commit
- Waiting for commit approval
- Push to remote
- Provide PR creation link
Changes Made:
-
library/sqlite/src/iosMain/kotlin/com/linroid/kdown/sqlite/DriverFactory.ios.kt:- Renamed constructor parameter from
dbPathtodbNameto clarify that only a database name (not a path) should be passed - Updated usage to pass
dbNamedirectly toNativeSqliteDriver
- Renamed constructor parameter from
-
app/shared/src/iosMain/kotlin/com/linroid/kdown/app/MainViewController.kt:- Simplified call site to pass just
"kdown.db"instead of constructing a full path - Removed unused
appSupportDbPath()function - Removed unused imports:
NSApplicationSupportDirectory,NSFileManager - Added clarifying comment about NativeSqliteDriver behavior
- Simplified call site to pass just
Next Steps:
The changes have been staged. Please approve the commit command in your terminal, then I'll push the changes and provide the PR link.
There was a problem hiding this comment.
@claude Last try: fix it and push to create PR, don't wait for my approval, I already approved. If your environment can't do that let me know I'll do it in terminal. Or can you send the session link, I can't find from claude.ai/code or github thread
There was a problem hiding this comment.
Claude finished @linroid's task in 1m 54s —— View job
Unable to commit without permission
I've implemented the iOS NativeSqliteDriver fix, but I cannot commit without permission approval in the workflow. The GitHub Actions environment requires approval for git commit commands.
Changes made:
library/sqlite/src/iosMain/kotlin/com/linroid/kdown/sqlite/DriverFactory.ios.kt: Renamed parameter fromdbPathtodbNamewith clarifying commentapp/shared/src/iosMain/kotlin/com/linroid/kdown/app/MainViewController.kt: Pass"kdown.db"directly, removed unused imports and helper function
Files are staged and ready to commit.
Since the environment blocks git commits, you'll need to run this in your terminal:
git commit -m "fix: use dbName parameter for iOS NativeSqliteDriver
NativeSqliteDriver only accepts a database filename (no path separators).
Renamed DriverFactory parameter from dbPath to dbName and updated iOS app
to pass 'kdown.db' directly instead of constructing a full path.
Fixes crash: File ... contains a path separator
Co-authored-by: Lin Zhang <linroid@users.noreply.github.com>"
git push origin claude/pr-58-20260213-0942Then create a PR with this URL:
Create PR →
There was a problem hiding this comment.
Seems claude bot can't push in a comment thread.
Fixed in #66
There was a problem hiding this comment.
I'm curious how did you find my project
I just searched in github: https://github.com/search?q=Kotlin%20Multiplatform%20download%20manager&type=repositories
I'm surprised there isn't more of a community of multiplatform libraries.
Take a look at my repo here: http://github.com/markst/radioplayer-kt
There was a problem hiding this comment.
Thanks for your information!
This library is still under active development, so I wouldn’t recommend using it i production until it becomes more stable.
2 participants
Summary
defaultDirectoryfield toDownloadConfigso the app layer provides platform-appropriate download paths instead of the hardcoded"downloads"relative pathDownloadCoordinatornow falls back toconfig.defaultDirectorywhenDownloadRequest.directoryis null, matching its KDoc contractDriverFactoryon JVM and iOS now requires an explicitdbPath— each app entry point provides the correct platform location:Environment.DIRECTORY_DOWNLOADSfor downloads; db viaContext(unchanged)~/Downloadsfor downloads;~/Library/Application Support/kdown/kdown.db~/Downloads;$XDG_CONFIG_HOME/kdown/kdown.db~/Downloads;%APPDATA%\kdown\kdown.dbApplication Support/kdown/kdown.dbTest plan
./gradlew :library:core:compileKotlinJvmpasses./gradlew :library:sqlite:compileKotlinJvmpasses./gradlew :library:core:jvmTestpasses./gradlew :app:android:compileDebugKotlinpasses./gradlew :app:desktop:compileKotlinpasses./gradlew :cli:compileKotlinpasses🤖 Generated with Claude Code