From 0e67f8e977bc5d86b641ebc5b6adbb7e1054ce1a Mon Sep 17 00:00:00 2001
From: CaYatur <cayatur@gmail.com>
Date: Tue, 28 Jul 2026 20:56:33 +0300
Subject: [PATCH] Self-review: make the bundled jar a real fallback, and see
 disabled jars
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

installBridge picked one source and committed to it, so the jar shipped with the
app only covered GitHub being unreachable. The commoner failure is the API
answering while the asset download does not — a dropped connection, or a proxy
that allows api.github.com but not objects.githubusercontent.com — and there the
install refused with a perfectly good jar on disk. That is what shipping the jar
is for.

installPlan() returns an ordered list and the installer walks it; a bad-asset
refusal falls through the same way, since "the release is wrong" is exactly when
the local copy is worth having. bridgeStatus reads the first step of the same
plan rather than repeating the ordering, so the version the warning names is the
version the button delivers.

installedJars matched only enabled jars, so a disabled bridge jar was invisible:
the status offered to install a version already sitting there, and the
post-install cleanup skipped it, so a server accumulated a disabled bridge per
version its operator ever turned off. Disabled jars are now found and cleaned
up; installedBridgeVersion still counts only enabled ones, because Bukkit does
not load one.
---
 src/main/core/bridgeInstall.ts | 106 ++++++++++++++++++---------------
 src/main/smoke.ts              |  22 +++++++
 src/shared/bridgeRelease.ts    |  31 ++++++++++
 3 files changed, 111 insertions(+), 48 deletions(-)

diff --git a/src/main/core/bridgeInstall.ts b/src/main/core/bridgeInstall.ts
index f29e6e5..670c78a 100644
--- a/src/main/core/bridgeInstall.ts
+++ b/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.
diff --git a/src/main/smoke.ts b/src/main/smoke.ts
index 8b4b0c4..34ec104 100644
--- a/src/main/smoke.ts
+++ b/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.
diff --git a/src/shared/bridgeRelease.ts b/src/shared/bridgeRelease.ts
index 6d57a6a..b6f7584 100644
--- a/src/shared/bridgeRelease.ts
+++ b/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