fix: open a device folder's workspace, and keep it (#383)

The Android picker is ACTION_OPEN_DOCUMENT_TREE and returns a directory,
never a file, so a workspace on device storage can only be reached through
the folder holding it. Nothing joined the two. The folder opened as a folder,
and the `.code-workspace` in it was reachable only by knowing to find the
file in the explorer and press the button on it; desktop VS Code offers that
step itself, from code the browser workbench does not carry. A granted folder
holding exactly one workspace file now opens as that workspace. Two is left
as a folder rather than guessed at, and a directory merely named like one is
excluded, because the workbench answers a workspace it cannot read with an
empty window and no message.

Pressing that button did not work either. It navigates the workbench to
`?workspace=<file>`, which the shell could not read: folderFromUrl parsed
only `folder` and required a directory, so both consumers of the page-load
callback were skipped. The workspace was not remembered and the next launch
fell through to the default projects directory, and adoptWorkbenchFolder
never ran, so a workspace opened out of a device-folder mirror got no
write-back watcher and every edit stayed in the mirror instead of reaching
the user's own files, with nothing on screen to say so.

workbenchUrl now names a workspace file with `workspace=` and everything else
with `folder=`, so a remembered workspace round-trips. folderFromUrl reads
both parameters through workbenchTarget, which stats each for the kind of
thing it is. The remembered path may now be a file, so its existence test is
exists() rather than isDirectory. resourceRootsInForce publishes the
directory holding a workspace rather than the file, which as a path prefix
would match only itself and refuse every resource beside it.

A closed folder was the same defect one state over. `?ew=true` is the third
thing the workbench can be showing and the folder chain cannot name it, so a
renderer crash over a closed folder reopened the workspace the user had just
closed. Those paths now restore the URL rather than rebuild one, which is
what handleResumeFromBackground already did for the same reason.

The saf-bridge extension also contributes Open Recent Folder to the status
bar remote indicator, the device picker having been reachable only by typing
its name into the command palette. Its directory moves to 1.5.0 because the
server caches the extension scan against the extensions root mtime, which
editing a manifest in place does not change.

The user guide said to open folders with File > Open Folder, which browses
the app's own storage and cannot reach Documents, Downloads or an SD card,
and FR-FS-05 claimed multi-root workspaces without qualification. Both now
say what the code does.

Verified on emulators at API 33 and API 37: granting Documents/wsdemo opens
it as `proj (Workspace)` with both roots, the edit that stayed in the mirror
now reaches the device folder, the workspace survives a cold start and a
renderer crash, a closed folder stays closed across one, and removing a
mirror while its workspace is open is still refused. A root the workspace
lists outside the granted folder is drawn in the warning colour rather than
failing silently, which is why none is guarded against.

Closes #381

Author: rmyndharis

MILESTONES.md

@@ -290,7 +290,7 @@ M6 (Release)   → Play Store release
      - Material Icon Theme, ESLint, Prettier, Python, Tailwind CSS
    - [x] 4 custom VSCodroid extensions:
      - `vscodroid.vscodroid-welcome-1.2.2`, welcome tab with quick actions
-     - `vscodroid.vscodroid-saf-bridge-1.4.0`, SAF storage integration
+     - `vscodroid.vscodroid-saf-bridge-1.5.0`, SAF storage integration
      - `vscodroid.vscodroid-process-monitor-1.2.0`, phantom process monitoring
      - `vscodroid.vscodroid-serve-network-1.1.0`, serve a dev server on the LAN
    - [x] `extensions.json` manifest auto-generated on first run

….vscodroid-saf-bridge-1.4.0/extension.js ….vscodroid-saf-bridge-1.5.0/extension.jsandroid/app/src/main/assets/extensions/vscodroid.vscodroid-saf-bridge-1.4.0/extension.js renamed to android/app/src/main/assets/extensions/vscodroid.vscodroid-saf-bridge-1.5.0/extension.js android/app/src/main/assets/extensions/vscodroid.vscodroid-saf-bridge-1.4.0/extension.js renamed to android/app/src/main/assets/extensions/vscodroid.vscodroid-saf-bridge-1.5.0/extension.js

@@ -2,7 +2,7 @@
   "name": "vscodroid-saf-bridge",
   "displayName": "VSCodroid Android Bridge",
   "description": "Commands that reach Android: device folders, browser, SSH keys, storage, about",
-  "version": "1.4.0",
+  "version": "1.5.0",
   "publisher": "vscodroid",
   "engines": {
     "vscode": "^1.96.0"
@@ -59,6 +59,10 @@
     ],
     "menus": {
       "statusBar/remoteIndicator": [
+        {
+          "command": "vscodroid.openRecentFolder",
+          "group": "remote_10_vscodroid_folder"
+        },
         {
           "command": "vscodroid.about",
           "group": "remote_90_vscodroid_about"

….vscodroid-saf-bridge-1.4.0/package.json ….vscodroid-saf-bridge-1.5.0/package.jsonandroid/app/src/main/assets/extensions/vscodroid.vscodroid-saf-bridge-1.4.0/package.json renamed to android/app/src/main/assets/extensions/vscodroid.vscodroid-saf-bridge-1.5.0/package.json android/app/src/main/assets/extensions/vscodroid.vscodroid-saf-bridge-1.4.0/package.json renamed to android/app/src/main/assets/extensions/vscodroid.vscodroid-saf-bridge-1.5.0/package.json

@@ -1230,9 +1230,21 @@ class MainActivity : AppCompatActivity() {
 
                 dialog.dismiss()
 
-                // Reload VS Code with the mirror directory
+                // Reload VS Code with the mirror, or with the workspace it holds.
+                // The listing is off the main thread for the reason every other
+                // disk read here is: `MainThreadWatch` installs a policy that
+                // logs one, and `folderOpenTarget` stats each candidate.
                 if (navigate && serverPort > 0) {
-                    navigateToFolder(serverPort, mirrorDir.absolutePath)
+                    val target = withContext(Dispatchers.IO) {
+                        folderOpenTarget(
+                            mirrorDir.absolutePath,
+                            mirrorDir.list()?.asList().orEmpty(),
+                        )
+                    }
+                    if (target != mirrorDir.absolutePath) {
+                        Logger.i(tag, "The granted folder holds a workspace; opening that")
+                    }
+                    navigateToFolder(serverPort, target)
                 }
             } catch (e: CancellationException) {
                 // Not a folder that failed. This Activity is being destroyed and
@@ -2303,12 +2315,22 @@ class MainActivity : AppCompatActivity() {
         }
     }
 
-    private fun loadVSCode(port: Int, folderPath: String? = null) {
+    private fun loadVSCode(port: Int, folderPath: String? = null, fromUrl: String? = null) {
         // Every route here is the user asking for the editor or the editor going
         // up over a page that never refused it, so the refusal is over. The one
         // caller that is neither checks the flag before calling; see onServerReady.
         rendererCrashLoopShown = false
         initBridge(port)
+        // Before the folder chain, because a closed folder is the one state that
+        // chain cannot name and would otherwise fall through to the remembered
+        // folder, reopening the workspace the user had just closed. [fromUrl] is
+        // for the caller whose WebView no longer holds the URL it is asking about,
+        // which is the renderer-crash path: [recreateWebView] builds a new one.
+        emptyWindowUrl(fromUrl ?: webView?.url, port)?.let {
+            Logger.i(tag, "Restoring the closed-folder window rather than a folder")
+            webView?.loadUrl(it)
+            return
+        }
         // onServerReady routes a restart through here without a folder. Falling back
         // to the folder already on screen keeps the user's workspace instead of
         // dropping them back into the default projects directory.
@@ -2387,8 +2409,14 @@ class MainActivity : AppCompatActivity() {
     private fun folderFromUrl(url: String?): String? =
         url?.let { it.toUri() }
             ?.takeIf { it.isHierarchical }
-            ?.getQueryParameter("folder")
-            ?.takeIf { File(it).isDirectory }
+            ?.let {
+                workbenchTarget(
+                    folder = it.getQueryParameter("folder"),
+                    workspace = it.getQueryParameter("workspace"),
+                    isDirectory = { path -> File(path).isDirectory },
+                    isFile = { path -> File(path).isFile },
+                )
+            }
 
     /**
      * Initializes the WebView bridge, security manager, and clients.
@@ -2675,7 +2703,11 @@ class MainActivity : AppCompatActivity() {
     private fun rememberedWorkspaceFolder(): String? = rememberedFolderToReopen(
         remembered = workspacePrefs.getString(KEY_LAST_FOLDER, null),
         mirrorsRoot = Environment.getSafMirrorsDir(this),
-        exists = { File(it).isDirectory },
+        // A directory or a `.code-workspace` file, because both are things the
+        // workbench can be pointed back at and both are now remembered. The test
+        // is still that the path is there: a workspace deleted while the app was
+        // away would otherwise be reopened onto nothing.
+        exists = { File(it).exists() },
         mirrorIsGranted = { safManager.folderForOpenedPath(it) != null },
     )
 
@@ -3537,7 +3569,7 @@ class MainActivity : AppCompatActivity() {
             // Always via loadVSCode so initBridge re-registers on the new WebView;
             // loading the old URL directly would leave it without the bridge. The
             // folder is carried over from the URL the destroyed WebView was showing.
-            loadVSCode(serverPort, folderFromUrl(lastUrl))
+            loadVSCode(serverPort, folderFromUrl(lastUrl), fromUrl = lastUrl)
         }
     }
 
@@ -4258,10 +4290,123 @@ internal fun authCallbackIsExpected(
  * from sending a wrong one.
  */
 internal fun workbenchUrl(port: Int, folderPath: String, token: String?): String {
-    val base = "http://127.0.0.1:$port/?folder=${Uri.encode(folderPath)}"
+    val param = if (folderPath.endsWith(WORKSPACE_FILE_SUFFIX)) "workspace" else "folder"
+    val base = "http://127.0.0.1:$port/?$param=${Uri.encode(folderPath)}"
     return if (token.isNullOrEmpty()) base else "$base&tkn=${Uri.encode(token)}"
 }
 
+/** What the workbench calls a multi-root workspace, in the only place it is spelled. */
+internal const val WORKSPACE_FILE_SUFFIX = ".code-workspace"
+
+/**
+ * What a workbench URL has open, whether that is a folder or a workspace.
+ *
+ * The workbench navigates itself, and it names the two cases with different
+ * query parameters: `folder` for a directory and `workspace` for a
+ * `.code-workspace` file. Reading only `folder` is what made a workspace
+ * invisible to this side, and the cost was not that the URL looked wrong. It was
+ * that [MainActivity.adoptWorkbenchFolder] never ran, so a workspace opened out
+ * of a device-folder mirror got no write-back watcher and every edit stayed in
+ * the mirror, and that the workspace was not remembered, so the next launch
+ * reopened the default projects directory instead.
+ *
+ * Each branch stats what it reads, and for the kind of thing it is. A `folder`
+ * naming a file and a `workspace` naming a directory are both URLs nothing
+ * builds; refusing them keeps a dead or wrong path from pinning the WebView,
+ * which is what the directory test has always been for.
+ *
+ * The predicates are passed rather than called because `File` is unavailable in
+ * a plain JVM test, the same reason [rememberedFolderToReopen] takes its own.
+ */
+internal fun workbenchTarget(
+    folder: String?,
+    workspace: String?,
+    isDirectory: (String) -> Boolean,
+    isFile: (String) -> Boolean,
+): String? = folder?.takeIf(isDirectory) ?: workspace?.takeIf(isFile)
+
+/**
+ * The directory a target stands in: itself for a folder, its parent for a workspace.
+ *
+ * Only the resource interceptor wants this. It publishes the open workspace as a
+ * resource root, and [com.vscodroid.webview.resourceRootsInForce] matches a root
+ * by path prefix, so a root that is a single `.code-workspace` file matches only
+ * that file and every resource beside it is refused. The statically published
+ * roots cover the mirrors and projects trees, so this is the difference only for
+ * a workspace held outside both, which is exactly the case nothing else covers.
+ *
+ * The other two readers of the field want the prefix and get it either way:
+ * `folderForOpenedPath` and `mirrorNameFor` both reduce a path to the mirror
+ * holding it, and a file inside a mirror reduces the same as its directory does.
+ */
+internal fun workspaceDirectoryInForce(path: String?): String? =
+    if (path != null && path.endsWith(WORKSPACE_FILE_SUFFIX)) File(path).parent else path
+
+/**
+ * What to open once a device folder has been granted and synced.
+ *
+ * The Android picker is `ACTION_OPEN_DOCUMENT_TREE` and returns a directory,
+ * never a file, so a workspace on device storage can only ever be reached
+ * through the folder holding it. Nothing joined the two: the folder opened as a
+ * folder, and the `.code-workspace` sitting in it was reachable only by knowing
+ * to find the file in the explorer and press the button on it. That is the gap
+ * behind "I cannot find a way to open an existing workspace", and it is the
+ * shell's to close, because desktop VS Code offers this from code the browser
+ * workbench does not carry (no `contains a workspace file` string exists in the
+ * bundle).
+ *
+ * Exactly one, at the top level, or nothing. Two is a guess, and guessing wrong
+ * is worse than opening the folder the user actually chose, from which either is
+ * one tap away. A directory that merely ends in `.code-workspace` is excluded
+ * because the workbench answers a workspace it cannot read with an empty window
+ * and no message, which is the failure this whole change exists to remove.
+ */
+/**
+ * The URL to reload when the workbench had the folder closed, or null.
+ *
+ * A closed folder is the third thing the workbench can be showing, and it is the
+ * one the folder chain cannot express: [workbenchTarget] answers a path or
+ * nothing, and "no folder, deliberately" is not a path. So every re-navigation
+ * over a closed folder fell through to the remembered folder and put the user
+ * back into the workspace they had just closed. `handleResumeFromBackground`
+ * already sidesteps this by calling `reload()` rather than rebuilding a URL, and
+ * this is the same answer for the paths that do rebuild.
+ *
+ * Returned verbatim and without a token. The workbench was already running, so
+ * the server has turned the token into a cookie that outlives this by a week,
+ * which is the reasoning the resume path states in full. A `folder` or
+ * `workspace` URL is deliberately NOT returned: those the folder chain can name,
+ * and it rebuilds them with a fresh token rather than reloading a stripped one.
+ *
+ * What makes a URL ours is asked of [workbenchUrl] rather than spelled again
+ * here. That keeps the host in exactly one expression, which is the affordance
+ * `the workbench URL is assembled in exactly one place` exists to protect: a
+ * second spelling is what lets the string loaded drift from the string logged,
+ * and it caught this function written the obvious way.
+ */
+internal fun emptyWindowUrl(url: String?, port: Int): String? {
+    if (url == null) return null
+    val ours = workbenchUrl(port, "", null).substringBefore('?')
+    if (!url.startsWith(ours)) return null
+    val query = url.substringAfter('?', "")
+    if (query.isEmpty()) return null
+    val closed = query.split('&').any {
+        it.substringBefore('=') == "ew" && it.substringAfter('=', "") == "true"
+    }
+    return if (closed) url else null
+}
+
+internal fun folderOpenTarget(
+    folderPath: String,
+    names: List<String>,
+    isFile: (String) -> Boolean = { File(it).isFile },
+): String =
+    names.filter { it.endsWith(WORKSPACE_FILE_SUFFIX) }
+        .map { "$folderPath${File.separator}$it" }
+        .filter(isFile)
+        .singleOrNull()
+        ?: folderPath
+
 /**
  * Whether a folder switch that failed should leave the previous folder watched.
  *

android/app/src/main/kotlin/com/vscodroid/MainActivity.kt

@@ -20,6 +20,7 @@ import com.vscodroid.bridge.authRequestIdsIn
 import com.vscodroid.isExtensionCallback
 import com.vscodroid.util.Environment
 import com.vscodroid.util.Logger
+import com.vscodroid.workspaceDirectoryInForce
 import java.io.ByteArrayInputStream
 import java.io.FilterInputStream
 import java.io.File
@@ -196,8 +197,15 @@ private var lastRefusedWorkspace: String? = null
  * See [lastRefusedWorkspace].
  */
 internal fun resourceRootsInForce(
-    published: List<String>, sensitive: List<String>, candidate: String?
+    published: List<String>, sensitive: List<String>, workspaceOrFolder: String?
 ): List<String> {
+    // Normalised here and not at the two suppliers, because both converge on this
+    // and `ServiceWorkerRetentionTest` refuses a supplier written as anything but
+    // `self.get()?....`: wrapping one would either capture the Activity or widen
+    // a check whose own message says not to. A `.code-workspace` names a file,
+    // and a root is matched by path prefix, so publishing the file publishes only
+    // itself and refuses every resource beside it.
+    val candidate = workspaceDirectoryInForce(workspaceOrFolder)
     val workspace = workspaceRootOrNull(candidate, sensitive)
     if (candidate != null && workspace == null) {
         if (candidate != lastRefusedWorkspace) {