Bridge installer: make the bundled jar a real fallback (#103 follow-up)

src/main/core/bridgeInstall.ts

@@ -24,6 +24,7 @@ import {
   bridgeSupported,
   bridgeVersionOf,
   compareBridgeVersions,
+  installPlan,
   isGithubAssetUrl,
   pickBridgeAsset,
   type BridgeAsset,
@@ -48,19 +49,24 @@ function pluginsDir(serverId: string): string {
  * and the operator sees "MSMS-Bridge could not be enabled" with no hint that
  * the cause is the jar they installed sitting next to the one they already had.
  */
-function installedJars(serverId: string): { name: string; version: string }[] {
+function installedJars(serverId: string): { name: string; version: string; enabled: boolean }[] {
   const dir = pluginsDir(serverId)
   if (!existsSync(dir)) return []
-  const out: { name: string; version: string }[] = []
+  const out: { name: string; version: string; enabled: boolean }[] = []
   for (const name of readdirSync(dir)) {
-    const version = bridgeVersionOf(name)
-    if (version) out.push({ name, version })
+    // A `.jar.disabled` is not loaded by Bukkit, so it is not installed as far
+    // as the status is concerned — but it IS a stale copy, and leaving one next
+    // to a newly installed jar means the folder accumulates a bridge per
+    // version an operator ever turned off.
+    const enabled = !/\.disabled$/i.test(name)
+    const version = bridgeVersionOf(enabled ? name : name.replace(/\.disabled$/i, ''))
+    if (version) out.push({ name, version, enabled })
   }
   return out.sort((a, b) => compareBridgeVersions(b.version, a.version))
 }
 
 export function installedBridgeVersion(serverId: string): string | null {
-  return installedJars(serverId)[0]?.version ?? null
+  return installedJars(serverId).find((j) => j.enabled)?.version ?? null
 }
 
 // ---- the copy that ships with the app ----
@@ -141,19 +147,11 @@ export async function bridgeStatus(serverId: string): Promise<BridgeStatus> {
   const bundled = bundledBridge()
   const offline = remote === null
 
-  let source: 'github' | 'bundled' | null = null
-  let latest: string | null = null
-  if (remote && bundled) {
-    const useRemote = compareBridgeVersions(remote.version, bundled.version) >= 0
-    source = useRemote ? 'github' : 'bundled'
-    latest = useRemote ? remote.version : bundled.version
-  } else if (remote) {
-    source = 'github'
-    latest = remote.version
-  } else if (bundled) {
-    source = 'bundled'
-    latest = bundled.version
-  }
+  // The same ordering the install will actually use, so the version the warning
+  // names is the version the button delivers.
+  const first = installPlan({ remote, bundled })[0] ?? null
+  const source = first
+  const latest = first === 'github' ? (remote?.version ?? null) : (bundled?.version ?? null)
 
   const need = bridgeNeed({ type: s.type, installed, latest })
   return { serverId, ...need, source: need.actionable ? source : null, ...(offline ? { offline: true } : {}) }
@@ -180,43 +178,55 @@ export async function installBridge(
 
   const remote = await latestBridge()
   const bundled = bundledBridge()
-  const preferRemote =
-    !!remote && (!bundled || compareBridgeVersions(remote.version, bundled.version) >= 0)
-  if (!remote && !bundled) return refuse(serverId, who, 'no-jar-available')
+  const plan = installPlan({ remote, bundled })
+  if (!plan.length) return refuse(serverId, who, 'no-jar-available')
 
   const dir = pluginsDir(serverId)
   mkdirSync(dir, { recursive: true })
 
-  let version: string
-  let written: string
-  let from: 'github' | 'bundled'
+  let version = ''
+  let written = ''
+  let from: 'github' | 'bundled' | null = null
+  let lastError = 'no-jar-available'
 
-  if (preferRemote && remote) {
-    // Re-checked here and not only where the asset was picked. This is the last
-    // line before a URL from a response body is handed to the downloader, and
-    // the two checks are cheap next to what passing a bad one would cost.
-    if (!isGithubAssetUrl(remote.url)) return refuse(serverId, who, 'bad-asset-url')
-    if (!BRIDGE_JAR_RE.test(remote.name)) return refuse(serverId, who, 'bad-asset-name')
-    written = join(dir, remote.name)
-    try {
-      await downloadFile(remote.url, written, {
-        ...(remote.sha256 ? { sha256: remote.sha256 } : {}),
-        timeoutMs: 60_000
-      })
-    } catch (e) {
-      rmSync(written, { force: true })
-      return refuse(serverId, who, 'download-failed: ' + String(e))
+  for (const step of plan) {
+    if (step === 'github' && remote) {
+      // Re-checked here and not only where the asset was picked. This is the
+      // last line before a URL from a response body is handed to the
+      // downloader, and the two checks are cheap next to what passing a bad one
+      // would cost. A refusal here does NOT abandon the install — it falls to
+      // the bundled jar like any other failure of this step.
+      if (!isGithubAssetUrl(remote.url) || !BRIDGE_JAR_RE.test(remote.name)) {
+        lastError = 'bad-asset'
+        continue
+      }
+      const dest = join(dir, remote.name)
+      try {
+        await downloadFile(remote.url, dest, {
+          ...(remote.sha256 ? { sha256: remote.sha256 } : {}),
+          timeoutMs: 60_000
+        })
+      } catch (e) {
+        // A half-written or hash-mismatched file must not be left behind: the
+        // next status check would read its name and report it as installed.
+        rmSync(dest, { force: true })
+        lastError = 'download-failed: ' + String(e)
+        continue
+      }
+      version = remote.version
+      written = dest
+      from = 'github'
+      break
+    }
+    if (step === 'bundled' && bundled) {
+      written = join(dir, bundled.name)
+      copyFileSync(bundled.path, written)
+      version = bundled.version
+      from = 'bundled'
+      break
     }
-    version = remote.version
-    from = 'github'
-  } else if (bundled) {
-    written = join(dir, bundled.name)
-    copyFileSync(bundled.path, written)
-    version = bundled.version
-    from = 'bundled'
-  } else {
-    return refuse(serverId, who, 'no-jar-available')
   }
+  if (!from) return refuse(serverId, who, lastError)
 
   // Only after the new jar is on disk. Removing first would leave a server with
   // no bridge at all if the download failed halfway.

src/main/smoke.ts

@@ -82,6 +82,7 @@ import {
   bridgeNeed,
   bridgeVersionOf,
   compareBridgeVersions,
+  installPlan,
   pickBridgeAsset,
   sha256Of
 } from '@shared/bridgeRelease'
@@ -1042,6 +1043,27 @@ export async function runModUpdateSmoke(): Promise<void> {
         if (need(t, null, '1.0.0') !== 'missing') return fail(t + ' should be offered the bridge')
       }
 
+      // What the install actually tries, in order. Committing to one source
+      // makes the bundled jar a fallback for exactly one failure — GitHub's API
+      // being unreachable — and leaves the commoner one uncovered: the API
+      // answering while the asset download fails, which would refuse the
+      // install with a perfectly good jar sitting on disk.
+      const plan = (r: string | null, b: string | null): string =>
+        installPlan({
+          remote: r ? { version: r } : null,
+          bundled: b ? { version: b } : null
+        }).join(',')
+      if (plan('1.2.0', '1.0.0') !== 'github,bundled') {
+        return fail('a failed download would not fall back to the bundled jar: ' + plan('1.2.0', '1.0.0'))
+      }
+      if (plan('1.0.0', '1.0.0') !== 'github,bundled') return fail('equal versions should still fall back')
+      // Downloading a jar older than the one on disk is work done to arrive
+      // somewhere worse.
+      if (plan('0.9.0', '1.0.0') !== 'bundled') return fail('an older release should not be downloaded')
+      if (plan('1.0.0', null) !== 'github') return fail('with no bundled jar there is nothing to fall back to')
+      if (plan(null, '1.0.0') !== 'bundled') return fail('with no release the bundled jar is the plan')
+      if (plan(null, null) !== '') return fail('with nothing available the plan must be empty')
+
       // The jar that ships with the app, which is the whole offline story. An
       // absent one would make every assertion above true and the feature
       // useless on the box a server manager actually runs on.

src/shared/bridgeRelease.ts

@@ -201,6 +201,37 @@ export function bridgeNeed(input: {
   return { state: 'ok', installed, ...(latest ? { latest } : {}), actionable: false }
 }
 
+/**
+ * What to try, in order, when installing.
+ *
+ * The obvious implementation picks one source and commits to it, and that turns
+ * the bundled jar into a fallback for exactly one failure — GitHub's API being
+ * unreachable — while leaving the more common one uncovered. The API answering
+ * and the asset download then failing (a CDN hiccup, a proxy that allows
+ * api.github.com but not objects.githubusercontent.com, a connection dropped
+ * mid-stream) would refuse the install with a perfectly good jar sitting on
+ * disk.
+ *
+ * So it is a list. An older bundled jar is still a working bridge, and a
+ * working bridge beats none; the result reports which source it came from, so
+ * nothing is claimed that is not true.
+ *
+ * `bundled` alone when it is the newer of the two: downloading a jar older than
+ * the one already on disk is work done to arrive somewhere worse.
+ */
+export function installPlan(o: {
+  remote?: { version: string } | null
+  bundled?: { version: string } | null
+}): ('github' | 'bundled')[] {
+  const { remote, bundled } = o
+  if (!remote && !bundled) return []
+  if (!remote) return ['bundled']
+  if (!bundled) return ['github']
+  return compareBridgeVersions(remote.version, bundled.version) >= 0
+    ? ['github', 'bundled']
+    : ['bundled']
+}
+
 /** What the panel, the desktop app and the API all receive. */
 export interface BridgeStatus extends BridgeNeed {
   serverId: string