Stop the staging fixture losing a race with the app-start sweep (#159)

`the sweep tolerates a staging path that is not a directory` failed once on run
33069641674, against 468 tests that pass on this machine including under
`--rerun-tasks`:

    java.io.FileNotFoundException at OutputPublisherStagingTest.kt:112
    468 tests completed, 1 failed

Line 112 was `writeBytes` immediately after `deleteRecursively()`.
`FileOutputStream` answers `FileNotFoundException` for an existing directory, so
something had recreated the path inside that window. That something is
`LibreMediaConverterApp.onCreate`, which ends with

    appScope.launch { OutputPublisher(...).sweepStaging() }

on `Dispatchers.IO`, and `sweepStaging` reads `stagingDir`, whose getter calls
`mkdirs()`. Robolectric builds the application for every test that asks for one,
so that background `mkdirs()` is in flight across the whole suite on a thread the
paused main looper does not control and no test awaits.

Retrying closes the window rather than narrowing it, because the race is not
symmetric: `mkdirs()` fails on an existing regular file, so the invariant only has
to survive being *established*. Once a write lands, nothing in the suite can turn
this path back into a directory -- which is also why the new assertion that the
sweep left a file behind is worth making.

The `check()` matters as much as the loop. The next failure here should say
"something recreated conversions/ as a directory", not `FileNotFoundException at
line 112` -- that is the difference between a flake someone reads and a flake
someone re-runs.

The wider problem is #159 and is deliberately not fixed here: `AppStartSweepTest`,
`JobSnapshotsTest` and `SpaceArithmeticTest` all name the same path, and the real
answer is an injectable scope rather than a retry loop in every staging test.
#159's done-when is that this loop can be deleted.

Mutation: `listFiles() ?: return` -> `listFiles()!!` reddens exactly this test.
Gate green: assembleDebug, testDebugUnitTest, compileDebugAndroidTestKotlin,
ktlintCheck, detekt, lintDebug.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

app/src/test/java/org/libremediaconverter/convert/FakeProviders.kt

@@ -0,0 +1,234 @@
+package org.libremediaconverter.convert
+
+import android.content.ComponentName
+import android.content.ContentProvider
+import android.content.ContentValues
+import android.content.Context
+import android.content.IntentFilter
+import android.content.pm.ProviderInfo
+import android.database.Cursor
+import android.database.MatrixCursor
+import android.net.Uri
+import android.os.Bundle
+import android.provider.DocumentsContract
+import android.provider.OpenableColumns
+import org.robolectric.Robolectric
+import org.robolectric.Shadows.shadowOf
+import java.io.File
+
+/**
+ * Content providers more than one test needs, and the registration dance they all repeat.
+ *
+ * Only that. A stub that serves one test stays in that test, next to the assertion it exists for —
+ * the rule `work/WorkerStubs.kt` states, and the reason `UnreliableOutputStream` is still private to
+ * `OutputPublisherPublishTest`.
+ *
+ * These started life inside `OutputPublisherPublishTest`, which is the only thing that needed a
+ * provider at all. They moved here when `InputQuery`'s cursor reads turned out to need the same
+ * provider answering *badly* — see [RowShape].
+ */
+
+internal const val DOCUMENTS_AUTHORITY = "org.libremediaconverter.test.documents"
+internal const val PLAIN_AUTHORITY = "org.libremediaconverter.test.plain"
+
+/**
+ * How [FakeSafProvider] answers a metadata query.
+ *
+ * A provider is another app. It can be uninstalled, revoke its grant, crash, or simply answer
+ * something the caller did not expect — and "answered something unexpected" is not one case but
+ * several, which is why this is an enum rather than a boolean.
+ *
+ * The distinction that matters most to callers is **null versus missing versus zero versus
+ * negative**. `InputQuery` exists to stop the last three being conflated: `hasSpaceFor(0)` is only
+ * "is there 128 MB free", so a size nobody could determine must not arrive as `0`, and
+ * `OutputPublisher.destinationIsKnownEmpty` must answer `false` — never "empty, go ahead and
+ * delete" — for every one of them.
+ *
+ * Column-level granularity is deliberate. `OutputPublisher` reads only `SIZE`; `InputQuery` reads
+ * both, and reaches a different answer depending on which one is bad.
+ */
+internal enum class RowShape {
+    /** What a healthy provider answers: the file's real name and real length. */
+    NORMAL,
+
+    /** A row is present and its `DISPLAY_NAME` cell is null. */
+    NULL_DISPLAY_NAME,
+
+    /** A row is present and its `SIZE` cell is null. */
+    NULL_SIZE,
+
+    /** The cursor carries no `DISPLAY_NAME` column at all — `getColumnIndex` gives `-1`. */
+    NO_DISPLAY_NAME_COLUMN,
+
+    /** The cursor carries no `SIZE` column at all — `getColumnIndex` gives `-1`. */
+    NO_SIZE_COLUMN,
+
+    /**
+     * A size of `-1`.
+     *
+     * Not a corrupt provider: it is what anything without a fixed length reports — a pipe, or a
+     * provider streaming its answer — and it is a third way of saying "unknown", distinct from a
+     * null cell and from a missing column.
+     */
+    NEGATIVE_SIZE,
+
+    /**
+     * A cursor with the right columns and no rows in it.
+     *
+     * Distinct from returning `null`, which is what a provider that does not recognise the URI
+     * does. Both mean "no answer", and code that treats one as an answer and the other as an
+     * absence is wrong about one of them.
+     */
+    NO_ROWS,
+
+    /**
+     * The query itself throws.
+     *
+     * A resolver call is a call into another app, and that app can have been uninstalled, revoked
+     * its grant, or simply crashed. `InputQuery.firstRow`'s KDoc is explicit that "a file picker is
+     * not a place to bring the process down from", so this is the shape that proves the guard is
+     * one.
+     */
+    QUERY_THROWS,
+}
+
+/**
+ * A stand-in for the provider behind a SAF destination.
+ *
+ * It answers only what its callers ask of a document -- how many bytes are already there, what it
+ * is called, and delete it -- backed by a real file so the assertions are about the filesystem
+ * rather than about a mock's call log alone. The rest of the `ContentProvider` surface is stubbed.
+ *
+ * Writing is deliberately NOT routed through it. Robolectric's `ShadowContentResolver`
+ * consults its registered-stream map before it reaches any provider, which is what lets a
+ * test hand out a stream that writes some bytes and then fails -- a condition a real provider
+ * cannot be asked to produce on demand.
+ */
+internal open class FakeSafProvider : ContentProvider() {
+
+    override fun onCreate() = true
+
+    override fun query(
+        uri: Uri,
+        projection: Array<out String>?,
+        selection: String?,
+        selectionArgs: Array<out String>?,
+        sortOrder: String?,
+    ): Cursor? {
+        if (rowShape == RowShape.QUERY_THROWS) throw SecurityException("provider revoked the grant")
+        val file = backingFile(uri)
+        if (!file.exists()) return null
+        return MatrixCursor(columnsFor(rowShape)).apply {
+            if (rowShape != RowShape.NO_ROWS) addRow(cellsFor(rowShape, file))
+        }
+    }
+
+    override fun call(method: String, arg: String?, extras: Bundle?): Bundle? {
+        if (method != METHOD_DELETE_DOCUMENT) return null
+        val target = extras?.getParcelable(EXTRA_URI, Uri::class.java) ?: return null
+        deleteRequests += target
+        deleteFailure?.let { throw it }
+        backingFile(target).delete()
+        return Bundle()
+    }
+
+    override fun getType(uri: Uri) = "video/mp4"
+
+    override fun insert(uri: Uri, values: ContentValues?): Uri? = null
+
+    override fun delete(uri: Uri, selection: String?, selectionArgs: Array<out String>?) = 0
+
+    override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<out String>?) = 0
+
+    companion object {
+        // DocumentsContract.METHOD_DELETE_DOCUMENT and EXTRA_URI are hidden from the public
+        // SDK, so they cannot be referenced. These are the wire names
+        // DocumentsContract.deleteDocument() actually sends, which is what a provider sees.
+        const val METHOD_DELETE_DOCUMENT = "android:deleteDocument"
+        const val EXTRA_URI = "uri"
+
+        /** Where the "documents" really live. Set per test to a Robolectric temp path. */
+        lateinit var root: File
+
+        /** Every delete this provider was asked for, in order. Empty is an assertion too. */
+        val deleteRequests = mutableListOf<Uri>()
+
+        /** Armed by the test that needs the cleanup itself to fail. */
+        var deleteFailure: RuntimeException? = null
+
+        /**
+         * How the next query answers. [reset] puts it back to [RowShape.NORMAL], so a test that
+         * does not care never has to think about it.
+         */
+        var rowShape: RowShape = RowShape.NORMAL
+
+        fun backingFile(uri: Uri) = File(root, uri.lastPathSegment.orEmpty())
+
+        fun reset(directory: File) {
+            root = directory
+            deleteRequests.clear()
+            deleteFailure = null
+            rowShape = RowShape.NORMAL
+        }
+
+        private fun columnsFor(shape: RowShape): Array<String> = when (shape) {
+            RowShape.NO_DISPLAY_NAME_COLUMN -> arrayOf(OpenableColumns.SIZE)
+            RowShape.NO_SIZE_COLUMN -> arrayOf(OpenableColumns.DISPLAY_NAME)
+            else -> arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE)
+        }
+
+        private fun cellsFor(shape: RowShape, file: File): Array<Any?> = when (shape) {
+            RowShape.NO_DISPLAY_NAME_COLUMN -> arrayOf(file.length())
+            RowShape.NO_SIZE_COLUMN -> arrayOf<Any?>(file.name)
+            RowShape.NULL_DISPLAY_NAME -> arrayOf(null, file.length())
+            RowShape.NULL_SIZE -> arrayOf(file.name, null)
+            RowShape.NEGATIVE_SIZE -> arrayOf(file.name, UNKNOWN_LENGTH)
+            else -> arrayOf(file.name, file.length())
+        }
+
+        /** What `statSize` reports for anything without a fixed length. See [RowShape.NEGATIVE_SIZE]. */
+        private const val UNKNOWN_LENGTH = -1L
+    }
+}
+
+/**
+ * The same provider, registered WITHOUT the documents-provider intent filter.
+ *
+ * A separate class because the package manager keys providers by component name, so two
+ * authorities need two components. It exists to prove the guard is a guard: a content URI
+ * from something that is not a documents provider must not be handed to `deleteDocument`.
+ */
+internal class FakePlainProvider : FakeSafProvider()
+
+/**
+ * Stands [provider] up on [authority] so `contentResolver` and the package manager both know it.
+ *
+ * `isDocumentUri()` does not look at the URI alone: it asks the package manager whether anything
+ * answers `ACTION_DOCUMENTS_PROVIDER` for that authority. Registering the provider with the
+ * resolver is not enough, which is the whole reason [asDocumentsProvider] is a parameter rather
+ * than always true — the negative case is a test.
+ */
+internal fun registerProvider(
+    context: Context,
+    provider: Class<out FakeSafProvider>,
+    authority: String,
+    asDocumentsProvider: Boolean,
+) {
+    val info = ProviderInfo().apply {
+        this.authority = authority
+        packageName = context.packageName
+        name = provider.name
+        exported = true
+        grantUriPermissions = true
+    }
+    Robolectric.buildContentProvider(provider).create(info)
+
+    val packageManager = shadowOf(context.packageManager)
+    packageManager.addOrUpdateProvider(info)
+    if (asDocumentsProvider) {
+        packageManager.addIntentFilterForProvider(
+            ComponentName(context.packageName, provider.name),
+            IntentFilter(DocumentsContract.PROVIDER_INTERFACE),
+        )
+    }
+}

app/src/test/java/org/libremediaconverter/convert/InputQueryCursorTest.kt

@@ -0,0 +1,187 @@
+package org.libremediaconverter.convert
+
+import android.content.Context
+import android.net.Uri
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.RuntimeEnvironment
+import java.io.File
+
+/**
+ * What [InputQuery] makes of a metadata row.
+ *
+ * ## Why this is a separate file from `UnknownInputSizeTest`
+ *
+ * That test drives the case where **no provider is registered** — the query returns null and
+ * `measure()` answers instead — and it drives it thoroughly. What it never does is hand `InputQuery`
+ * a row. Before this file, nothing did: `firstRow`'s body, `displayNameOrNull` and `sizeOrNull` had
+ * never executed in the JVM suite, so every branch inside them was untested.
+ *
+ * ## What is actually being pinned
+ *
+ * Not "does it read a cursor" — that would pass against almost any implementation. The rule is that
+ * **a size nobody could determine must not arrive as a number**, and there are four separate ways a
+ * provider fails to determine one: a null cell, a missing column, a negative value, and no row at
+ * all. `InputQuery`'s KDoc states the stake:
+ *
+ * > a worker's input `Data` carries the size the *picker* found … `hasSpaceFor(0)` is only "is there
+ * > 128 MB free".
+ *
+ * So each of those four must produce `null`, and `null` specifically — not `0`, not `-1`. A test
+ * that asserted only "not the file's length" would pass on `0`, which is the exact conflation the
+ * class exists to end.
+ *
+ * ## Why every fall-through lands on null here
+ *
+ * [FakeSafProvider] does not implement `openFile`, so `measure()` cannot answer for these URIs
+ * either. That is deliberate: it isolates the cursor half. The other direction — the cursor says
+ * nothing and `measure()` succeeds — is `UnknownInputSizeTest`'s
+ * `a picked file no provider describes is measured rather than reported as empty`, and is not
+ * repeated here.
+ *
+ * ## What the mutations say, including the one that does not bite
+ *
+ * Measured against `MatrixCursor`, which is what these tests drive:
+ *
+ * | call on a null cell | result |
+ * |---|---|
+ * | `getString` | returns `null` |
+ * | `getLong` | returns **`0`** |
+ *
+ * That second row is why `sizeOrNull`'s `!isNull(it)` guard is load-bearing and why these tests
+ * bite: remove it and a null size arrives as `0`, a real number indistinguishable from an empty
+ * file, which is the precise conflation this class exists to end. Removing it reddens
+ * `a null size is unknown rather than zero`. Removing the trailing `takeIf { it >= 0 }` reddens
+ * `a negative size is unknown rather than reported`.
+ *
+ * **Named exemption: `displayNameOrNull`'s `!isNull(it)` guard is not pinned by anything here, and
+ * cannot be.** `getString` returns null for a null cell, so the fallback applies with or without
+ * the guard — removing it leaves every test in this file green. The guard is not redundant in
+ * production: `Cursor.getString`'s contract states that whether it throws on a null column is
+ * *implementation-defined*, and a real `ContentProvider` is free to throw where `MatrixCursor`
+ * returns null. It should stay. It simply cannot be falsified with this cursor, and saying so is
+ * better than implying `a null display name falls back without disturbing the size` covers it —
+ * that test pins the behaviour, not the guard.
+ */
+@RunWith(RobolectricTestRunner::class)
+class InputQueryCursorTest {
+
+    private lateinit var context: Context
+    private lateinit var uri: Uri
+
+    @Before
+    fun setUp() {
+        context = RuntimeEnvironment.getApplication()
+        FakeSafProvider.reset(File(context.cacheDir, "picked").apply { mkdirs() })
+        registerProvider(context, FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
+        uri = Uri.parse("content://$DOCUMENTS_AUTHORITY/document/holiday.mp4")
+        FakeSafProvider.backingFile(uri).writeBytes(ByteArray(PAYLOAD_BYTES))
+    }
+
+    @Test
+    fun `a provider that answers properly supplies both the name and the size`() {
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals("holiday.mp4", described.displayName)
+        assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
+    }
+
+    @Test
+    fun `a null display name falls back without disturbing the size`() {
+        FakeSafProvider.rowShape = RowShape.NULL_DISPLAY_NAME
+
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
+        // The two columns are read independently. A provider that cannot name the file can still
+        // size it, and losing the size here would be a bug the name assertion alone would miss.
+        assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
+    }
+
+    @Test
+    fun `a cursor with no display name column falls back rather than throwing`() {
+        // getColumnIndex returns -1 rather than throwing, so the `it >= 0` guard is the only thing
+        // between this and an IllegalArgumentException out of getString.
+        FakeSafProvider.rowShape = RowShape.NO_DISPLAY_NAME_COLUMN
+
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
+        assertEquals(PAYLOAD_BYTES.toLong(), described.sizeBytes)
+    }
+
+    @Test
+    fun `a null size is unknown rather than zero`() {
+        FakeSafProvider.rowShape = RowShape.NULL_SIZE
+
+        assertNull(unknownSizeMessage("a null cell"), InputQuery.sizeOf(context, uri))
+    }
+
+    @Test
+    fun `a cursor with no size column is unknown rather than zero`() {
+        FakeSafProvider.rowShape = RowShape.NO_SIZE_COLUMN
+
+        assertNull(unknownSizeMessage("a missing column"), InputQuery.sizeOf(context, uri))
+    }
+
+    @Test
+    fun `a negative size is unknown rather than reported`() {
+        // What anything without a fixed length reports -- a pipe, or a provider streaming its
+        // answer. Passing -1 through would be worse than passing 0: hasSpaceFor compares it
+        // against free space, so it would read as "needs less than nothing".
+        FakeSafProvider.rowShape = RowShape.NEGATIVE_SIZE
+
+        assertNull(unknownSizeMessage("a negative size"), InputQuery.sizeOf(context, uri))
+    }
+
+    @Test
+    fun `a cursor with no rows is unknown rather than zero`() {
+        // Distinct from the provider returning null, which UnknownInputSizeTest covers. A cursor
+        // that exists and holds nothing still has to reach the same answer.
+        FakeSafProvider.rowShape = RowShape.NO_ROWS
+
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
+        assertNull(unknownSizeMessage("an empty cursor"), described.sizeBytes)
+    }
+
+    @Test
+    fun `a provider that throws is survived rather than propagated`() {
+        // The guard firstRow's KDoc exists for: "a resolver call is a call into another app ... and
+        // a file picker is not a place to bring the process down from". Without the runCatching,
+        // this SecurityException reaches the caller and takes the pick with it.
+        FakeSafProvider.rowShape = RowShape.QUERY_THROWS
+
+        val described = InputQuery.describe(context, uri)
+
+        assertEquals(InputQuery.FALLBACK_DISPLAY_NAME, described.displayName)
+        assertNull(unknownSizeMessage("a provider that threw"), described.sizeBytes)
+    }
+
+    @Test
+    fun `a join total is unknown when any one input could not be sized`() {
+        // The consequence the four cases above exist for, asserted once at the place it lands.
+        // Summing the inputs that did answer would produce a lower bound indistinguishable from a
+        // real total, which is what the space check cannot tell apart.
+        FakeSafProvider.rowShape = RowShape.NULL_SIZE
+        val unsizable = InputQuery.sizeOf(context, uri)
+        FakeSafProvider.rowShape = RowShape.NORMAL
+        val sizable = InputQuery.sizeOf(context, uri)
+
+        assertEquals(PAYLOAD_BYTES.toLong(), sizable)
+        assertNull(unsizable)
+        assertNull("one unknown input makes the whole total unknown", InputQuery.total(listOf(sizable, unsizable)))
+    }
+
+    private fun unknownSizeMessage(cause: String) =
+        "$cause means nobody could size the file; that must be null, not 0 -- hasSpaceFor(0) is only a headroom check"
+
+    private companion object {
+        const val PAYLOAD_BYTES = 4096
+    }
+}

app/src/test/java/org/libremediaconverter/convert/OutputPublisherPublishTest.kt

@@ -1,17 +1,7 @@
 package org.libremediaconverter.convert
 
-import android.content.ComponentName
-import android.content.ContentProvider
-import android.content.ContentValues
 import android.content.Context
-import android.content.IntentFilter
-import android.content.pm.ProviderInfo
-import android.database.Cursor
-import android.database.MatrixCursor
 import android.net.Uri
-import android.os.Bundle
-import android.provider.DocumentsContract
-import android.provider.OpenableColumns
 import org.junit.Assert.assertArrayEquals
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
@@ -20,7 +10,6 @@ import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.robolectric.Robolectric
 import org.robolectric.RobolectricTestRunner
 import org.robolectric.RuntimeEnvironment
 import org.robolectric.Shadows.shadowOf
@@ -31,90 +20,8 @@ import java.io.OutputStream
 /** What a destination volume says when it fills up mid-write. */
 private const val NO_SPACE = "No space left on device"
 
-private const val DOCUMENTS_AUTHORITY = "org.libremediaconverter.test.documents"
-private const val PLAIN_AUTHORITY = "org.libremediaconverter.test.plain"
-
-/**
- * A stand-in for the provider behind a SAF destination.
- *
- * It answers only what `publish()` asks of a destination -- how many bytes are already there,
- * and delete it -- backed by a real file so the assertions are about the filesystem rather
- * than about a mock's call log alone. The rest of the `ContentProvider` surface is stubbed.
- *
- * Writing is deliberately NOT routed through it. Robolectric's `ShadowContentResolver`
- * consults its registered-stream map before it reaches any provider, which is what lets a
- * test hand out a stream that writes some bytes and then fails -- the condition this whole
- * file exists for, and one a real provider cannot be asked to produce on demand.
- */
-internal open class FakeSafProvider : ContentProvider() {
-
-    override fun onCreate() = true
-
-    override fun query(
-        uri: Uri,
-        projection: Array<out String>?,
-        selection: String?,
-        selectionArgs: Array<out String>?,
-        sortOrder: String?,
-    ): Cursor? {
-        val file = backingFile(uri)
-        if (!file.exists()) return null
-        return MatrixCursor(arrayOf(OpenableColumns.DISPLAY_NAME, OpenableColumns.SIZE)).apply {
-            addRow(arrayOf<Any?>(file.name, file.length()))
-        }
-    }
-
-    override fun call(method: String, arg: String?, extras: Bundle?): Bundle? {
-        if (method != METHOD_DELETE_DOCUMENT) return null
-        val target = extras?.getParcelable(EXTRA_URI, Uri::class.java) ?: return null
-        deleteRequests += target
-        deleteFailure?.let { throw it }
-        backingFile(target).delete()
-        return Bundle()
-    }
-
-    override fun getType(uri: Uri) = "video/mp4"
-
-    override fun insert(uri: Uri, values: ContentValues?): Uri? = null
-
-    override fun delete(uri: Uri, selection: String?, selectionArgs: Array<out String>?) = 0
-
-    override fun update(uri: Uri, values: ContentValues?, selection: String?, selectionArgs: Array<out String>?) = 0
-
-    companion object {
-        // DocumentsContract.METHOD_DELETE_DOCUMENT and EXTRA_URI are hidden from the public
-        // SDK, so they cannot be referenced. These are the wire names
-        // DocumentsContract.deleteDocument() actually sends, which is what a provider sees.
-        const val METHOD_DELETE_DOCUMENT = "android:deleteDocument"
-        const val EXTRA_URI = "uri"
-
-        /** Where the "documents" really live. Set per test to a Robolectric temp path. */
-        lateinit var root: File
-
-        /** Every delete this provider was asked for, in order. Empty is an assertion too. */
-        val deleteRequests = mutableListOf<Uri>()
-
-        /** Armed by the test that needs the cleanup itself to fail. */
-        var deleteFailure: RuntimeException? = null
-
-        fun backingFile(uri: Uri) = File(root, uri.lastPathSegment.orEmpty())
-
-        fun reset(directory: File) {
-            root = directory
-            deleteRequests.clear()
-            deleteFailure = null
-        }
-    }
-}
-
-/**
- * The same provider, registered WITHOUT the documents-provider intent filter.
- *
- * A separate class because the package manager keys providers by component name, so two
- * authorities need two components. It exists to prove the guard is a guard: a content URI
- * from something that is not a documents provider must not be handed to `deleteDocument`.
- */
-internal class FakePlainProvider : FakeSafProvider()
+/** How far a failing copy gets before the volume "fills up". Any value below the payload does. */
+private const val PARTIAL_BYTES = 512
 
 /**
  * A sink that behaves like a volume filling up.
@@ -171,6 +78,8 @@ class OutputPublisherPublishTest {
 
     private val payload = ByteArray(8192) { (it % 251).toByte() }
 
+    /** How far a failing copy gets before the volume "fills up". Any value below the payload does. */
+
     private val documentUri: Uri = Uri.parse("content://$DOCUMENTS_AUTHORITY/document/holiday.mp4")
     private val plainUri: Uri = Uri.parse("content://$PLAIN_AUTHORITY/document/holiday_plain.mp4")
     private val deadUri: Uri = Uri.parse("content://org.libremediaconverter.nonexistent/document/gone.mp4")
@@ -179,8 +88,8 @@ class OutputPublisherPublishTest {
     fun setUp() {
         context = RuntimeEnvironment.getApplication()
         FakeSafProvider.reset(File(context.cacheDir, "destinations").apply { mkdirs() })
-        register(FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
-        register(FakePlainProvider::class.java, PLAIN_AUTHORITY, asDocumentsProvider = false)
+        registerProvider(context, FakeSafProvider::class.java, DOCUMENTS_AUTHORITY, asDocumentsProvider = true)
+        registerProvider(context, FakePlainProvider::class.java, PLAIN_AUTHORITY, asDocumentsProvider = false)
 
         // SAF's CreateDocument contract hands back a document that already exists and is
         // empty, so that is the state every destination starts in here.
@@ -299,6 +208,50 @@ class OutputPublisherPublishTest {
         assertEquals(emptyList<Uri>(), FakeSafProvider.deleteRequests)
     }
 
+    @Test
+    fun `a destination whose size cannot be determined is never deleted`() {
+        // The three short-circuits in destinationIsKnownEmpty, and the reason its KDoc gives for
+        // each of them answering false:
+        //
+        //   "this decides whether a delete is allowed and 'I could not tell' must never authorise
+        //    one."
+        //
+        // The contrast is `a copy that fails partway leaves nothing at the destination` above: a
+        // provider that *does* say zero gets the delete. These say nothing, so they must not.
+        // Getting this backwards costs the user a file they already had, on a save that failed.
+        //
+        // Named exemption: of the three conjuncts, `size >= 0` cannot be falsified behaviourally.
+        // Measured -- getColumnIndex returns -1 for an absent column, and isNull(-1) throws
+        // CursorIndexOutOfBoundsException, which the surrounding runCatching already turns into
+        // `?: false`. So relaxing it to `size >= -1` leaves this test green: same answer, reached
+        // by the exception path instead. The guard should stay -- control flow through an exception
+        // is worse than a comparison, and another Cursor implementation need not throw -- but no
+        // assertion here pins it, and saying so beats implying the missing-column case covers it.
+        // `!row.isNull(size)` and `row.moveToFirst()` do both bite.
+        listOf(
+            RowShape.NO_SIZE_COLUMN to "a cursor with no SIZE column",
+            RowShape.NULL_SIZE to "a cursor whose SIZE cell is null",
+            RowShape.NO_ROWS to "a cursor holding no rows",
+        ).forEach { (shape, description) ->
+            FakeSafProvider.deleteRequests.clear()
+            FakeSafProvider.backingFile(documentUri).writeBytes(ByteArray(0))
+            FakeSafProvider.rowShape = shape
+            failMidCopy(documentUri, afterBytes = PARTIAL_BYTES)
+
+            assertThrows(IOException::class.java) { publisher.publish(staged, documentUri) }
+
+            assertEquals(
+                "$description must not authorise a delete",
+                emptyList<Uri>(),
+                FakeSafProvider.deleteRequests,
+            )
+            assertTrue(
+                "$description must leave the destination where it was",
+                FakeSafProvider.backingFile(documentUri).exists(),
+            )
+        }
+    }
+
     @Test
     fun `a copy that succeeds delivers every byte and deletes nothing`() {
         shadowOf(context.contentResolver).registerOutputStreamSupplier(documentUri) {
@@ -326,28 +279,4 @@ class OutputPublisherPublishTest {
             )
         }
     }
-
-    private fun register(provider: Class<out FakeSafProvider>, authority: String, asDocumentsProvider: Boolean) {
-        val info = ProviderInfo().apply {
-            this.authority = authority
-            packageName = context.packageName
-            name = provider.name
-            exported = true
-            grantUriPermissions = true
-        }
-        Robolectric.buildContentProvider(provider).create(info)
-
-        // isDocumentUri() does not look at the URI alone: it asks the package manager whether
-        // anything answers ACTION_DOCUMENTS_PROVIDER for that authority. Registering the
-        // provider with the resolver is not enough, which is the whole reason the negative
-        // case above can exist.
-        val packageManager = shadowOf(context.packageManager)
-        packageManager.addOrUpdateProvider(info)
-        if (asDocumentsProvider) {
-            packageManager.addIntentFilterForProvider(
-                ComponentName(context.packageName, provider.name),
-                IntentFilter(DocumentsContract.PROVIDER_INTERFACE),
-            )
-        }
-    }
 }

app/src/test/java/org/libremediaconverter/convert/OutputPublisherStagingTest.kt

@@ -1,6 +1,7 @@
 package org.libremediaconverter.convert
 
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
@@ -99,4 +100,71 @@ class OutputPublisherStagingTest {
 
         publisher.sweepStaging()
     }
+
+    @Test
+    fun `the sweep tolerates a staging path that is not a directory`() {
+        // The other half of `listFiles() ?: return`, and not the same as the case above: a missing
+        // directory is created by `stagingDir`'s own mkdirs() and lists as empty. Only a path that
+        // cannot be a directory makes listFiles() answer null, and a sweep that dereferenced that
+        // would take the app down on a launch rather than on a conversion -- AppStartSweepTest is
+        // where this runs from.
+        val stagingPath = stagingPathAsRegularFile()
+
+        publisher.sweepStaging()
+
+        assertTrue("the sweep must not have replaced the fixture", stagingPath.isFile)
+    }
+
+    /**
+     * Makes `cacheDir/conversions` a regular file, which is the whole precondition of the test
+     * above -- and does it in a loop, because a single delete-then-write loses a race that CI
+     * caught and this machine does not reproduce.
+     *
+     * `LibreMediaConverterApp.onCreate` ends with
+     * `appScope.launch { OutputPublisher(...).sweepStaging() }` on `Dispatchers.IO`, and
+     * `sweepStaging` reads `stagingDir`, whose getter calls `mkdirs()`. Robolectric instantiates
+     * the application for every test that asks for one, so that background `mkdirs()` is in flight
+     * across the whole suite, on a thread the paused main looper does not control. Between deleting
+     * this path and writing it there is a window where the path does not exist and that `mkdirs()`
+     * can win, which is `FileNotFoundException: ... (Is a directory)` out of `writeBytes` -- run
+     * 33069641674 on #149, once, against 468 tests that pass here.
+     *
+     * Retrying closes it rather than narrowing it, because the race is not symmetric: `mkdirs()`
+     * fails on an existing regular file, so the invariant only has to survive being *established*.
+     * Once a write lands, nothing in the suite can turn this back into a directory.
+     *
+     * The wider problem -- application-scope IO work racing every Robolectric test that shares
+     * `cacheDir` -- is #159, and is deliberately not fixed here.
+     */
+    private fun stagingPathAsRegularFile(): File {
+        val stagingPath = File(cacheDir, "conversions")
+        repeat(FIXTURE_ATTEMPTS) {
+            if (stagingPath.isFile) return stagingPath
+            stagingPath.deleteRecursively()
+            runCatching { stagingPath.writeBytes(ByteArray(FIXTURE_BYTES)) }
+        }
+        check(stagingPath.isFile) {
+            "the fixture needs $stagingPath to be a regular file and it is a directory; " +
+                "something recreated it $FIXTURE_ATTEMPTS times -- see #159"
+        }
+        return stagingPath
+    }
+
+    @Test
+    fun `discarding a file with no parent at all is refused`() {
+        // A relative name has no parent directory, so `staged.parentFile` is null. The handle
+        // reaches the ViewModel as a path string out of WorkInfo.outputData and is turned straight
+        // into a File, so this is not a shape the caller can rule out -- and the guard has to
+        // answer false rather than dereference it.
+        val parentless = File("holiday.mp4")
+        assertNull("the fixture is supposed to have no parent", parentless.parentFile)
+
+        assertFalse("a file with no parent is not in staging", publisher.discardStaged(parentless))
+    }
+
+    private companion object {
+        /** Enough to outlast a burst of application-scope sweeps; one attempt is what CI lost. */
+        const val FIXTURE_ATTEMPTS = 50
+        const val FIXTURE_BYTES = 8
+    }
 }