fix/ADFA-3923 ensure temporary file deletion after install#1273
fix/ADFA-3923 ensure temporary file deletion after install#1273davidschachterADFA merged 3 commits intostagefrom
Conversation
📝 WalkthroughRelease Notes
|
| Layer / File(s) | Summary |
|---|---|
Directory Preparation plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt |
Calls ensureDirectory(destination) to guarantee the extraction target exists. |
Stale-artifact Cleanup plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt |
Removes files matching archive*.tar.xz from destination.parentFile before extraction; logs a warning on cleanup failure. |
Temp File Creation & Marking plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt |
Creates a temporary .tar.xz file in the destination parent and calls tempFile.deleteOnExit(). |
Buffered Write plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt |
Writes incoming data into the temp file using a buffered copyTo instead of copying directly from the original source. |
Extraction Invocation plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt |
Delegates extraction to extractTarXzViaTermux using the created temp file. |
Cleanup / Failure Handling plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt |
In finally, attempts to delete the temp file; if deletion fails, logs a warning and ensures deleteOnExit() is set as a fallback. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested reviewers
- Daniel-ADFA
- davidschachterADFA
Poem
🐰 In a burrow of bytes I gently hop,
I buffer the stream and tidy the crop,
Temp files I plant, then wave them goodbye,
If delete fails, I schedule the sky —
Soft logs whisper, "we'll clean up by and by."
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main change: ensuring temporary file deletion after NDK plugin installation, which aligns with the changeset's focus on improved cleanup in the TAR_XZ extraction path. |
| Description check | ✅ Passed | The description is directly related to the changeset, specifying the intent to ensure deletion of archive*.tar.xz files after NDK plugin installation, which matches the changes in IdeArchiveServiceImpl. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/ADFA-3923-ndk-postinstall
Tip
💬 Introducing Slack Agent: The best way for teams to turn conversations into code.
Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
- Generate code and open pull requests
- Plan features and break down work
- Investigate incidents and troubleshoot customer tickets together
- Automate recurring tasks and respond to alerts with triggers
- Summarize progress and report instantly
Built for teams:
- Shared memory across your entire org—no repeating context
- Per-thread sandboxes to safely plan and execute work
- Governance built-in—scoped access, auditability, and budget controls
One agent for your entire SDLC. Right inside Slack.
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt`:
- Around line 45-55: In IdeArchiveServiceImpl's TAR_XZ extraction path, don't
rely on tempFile.deleteOnExit(); before creating tempFile, scan
destination.parentFile for leftover files matching "archive*.tar.xz" (or the
same naming pattern used to create tempFile), attempt to delete each stale file
and log any failures via logger, then proceed to create the new tempFile and
call extractTarXzViaTermux; keep the existing tempFile.delete() cleanup and
remove/avoid relying on deleteOnExit() as the primary fallback, and ensure all
delete attempts are wrapped to catch and log exceptions so extraction still
proceeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a02943b7-2040-453e-996d-d41c8d6610f3
📒 Files selected for processing (1)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt
jomen-adfa
force-pushed
the
fix/ADFA-3923-ndk-postinstall
branch
from
7601c43 to
0cedb61
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt (1)
57-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
copyInterruptible()for the TAR_XZ staging step.The TAR_XZ branch is the only archive format that bypasses interrupt checking and progress reporting during the initial copy to disk. All other formats (ZIP, TAR, TAR_GZ, XZ, GZIP) use
copyInterruptible(), which provides both cancellation support and periodiconProgresscallbacks. For large TAR_XZ archives, the staging copy can block indefinitely with no UI feedback.Suggested fix
- tempFile.outputStream().use { buffered.copyTo(it) } + tempFile.outputStream().use { out -> + copyInterruptible(buffered, out, onProgress, entryName = tempFile.name) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt` around lines 57 - 58, The TAR_XZ staging currently writes with buffered.copyTo(it) which skips cancellation and progress; replace that call with the same interruptible copy used by other branches (use copyInterruptible(...) instead of copyTo) inside the tempFile.outputStream().use block so the staging copy honors cancellation and emits onProgress updates, keeping the surrounding logic that calls extractTarXzViaTermux(destination) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt`:
- Around line 45-54: The code currently lists and deletes "archive*.tar.xz"
files in destination.parentFile and creates tempFile there — instead restrict
both the sweep and temp creation to the validated extraction root by using
destination (or its canonical directory) instead of destination.parentFile; e.g.
compute val extractionRoot = destination.canonicalFile (or
destination.canonicalFile.parentFile if destination is a file) and only
list/delete files under extractionRoot while verifying each
stale.canonicalPath.startsWith(extractionRoot.canonicalPath) before deleting,
and create the temp with File.createTempFile("archive", ".tar.xz",
extractionRoot) (update references to tempFile, stale, and the logger.warn usage
accordingly).
---
Duplicate comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt`:
- Around line 57-58: The TAR_XZ staging currently writes with
buffered.copyTo(it) which skips cancellation and progress; replace that call
with the same interruptible copy used by other branches (use
copyInterruptible(...) instead of copyTo) inside the tempFile.outputStream().use
block so the staging copy honors cancellation and emits onProgress updates,
keeping the surrounding logic that calls extractTarXzViaTermux(destination)
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4a5c7832-3047-4e1c-b672-68c01761577d
📒 Files selected for processing (1)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeArchiveServiceImpl.kt
2 participants
ensure deletion of %HOME/archive*.tar.xz files after installation of NDK plugin