Enforce cancellation-safe runCatching: runCatchingCancellable + SwallowedCancellation lint check

CLAUDE.md

@@ -218,6 +218,24 @@ follow that pattern when adding server-domain time math.
   mints into `WallTime`/`ElapsedTime` (domain made explicit) or carries an inline `@Suppress` with a
   one-line rationale (there is no lint baseline — these live at the site). See `lint-rules/README.md`.
 
+## Coroutines: never swallow CancellationException (#1908 / #1921)
+
+`kotlin.runCatching` catches **every** `Throwable`, and a broad `catch (Exception | Throwable |
+RuntimeException | IllegalStateException)` catches `CancellationException` too — it's a plain
+`java.util.concurrent.CancellationException`, i.e. an `IllegalStateException` subtype. So in coroutine code
+either one silently converts a cancelled coroutine's `CancellationException` into a `Result.failure` /
+caught value: the cancelled work keeps running and callers read the cancellation as an ordinary
+error/empty state (structured concurrency breaks). It passes compilation and review and only misbehaves
+under real cancellation.
+
+- **Use `runCatchingCancellable`** (`org.onebusaway.android.util`) instead of bare `runCatching` in any
+  `suspend`/coroutine code. It's behaviour-identical but rethrows `CancellationException`. The custom lint
+  check `SwallowedCancellation` (module `:lint-rules`, no baseline) fails the build on a bare `runCatching`
+  in a `suspend` function, so this is the enforced path.
+- For a broad `try/catch` in coroutine code, put a leading `catch (e: CancellationException) { throw e }`
+  before the broad catch (the lint check covers `runCatching` only). Narrow catches (`IOException`, …)
+  don't catch cancellation and are fine.
+
 ## No unsanctioned heuristics
 
 Do **not** introduce heuristics — magic thresholds, magnitude guesses, or "good enough" inference of

lint-rules/README.md

@@ -81,6 +81,29 @@ is a property of the *endpoint* — the two same-named `StopTime` (seconds) and
 millis) DTOs are the proof — so only the endpoint-aware adapter can mint correctly, and reading the raw
 field elsewhere, even to pass it straight on, is the escape.
 
+### `SwallowedCancellation` (warning)
+Fires when `kotlin.runCatching` is called inside a `suspend` function. `runCatching` catches every
+`Throwable`, so in a coroutine it also swallows the `CancellationException` a cancelled coroutine throws
+from a suspend call — converting cancellation into an ordinary `Result.failure`, so the cancelled work
+keeps running and callers read the cancellation as a normal error/empty state (structured concurrency
+breaks). `CancellationException` is a plain `IllegalStateException` subtype, so nothing at the type level
+catches this; it passes compilation and review and only misbehaves under real cancellation (screen closed,
+search superseded by a keystroke, poll tick outrun). This is the #1908 / #1921 bug class.
+
+Fix by using **`runCatchingCancellable`** (`org.onebusaway.android.util`) — behaviour-identical to
+`runCatching` but it rethrows `CancellationException` (a real failure still resolves to `Result.failure`).
+The wrapper is the single sanctioned path, so the check degrades to a symbol ban: any bare `runCatching`
+in a suspend function is flagged. For a broad `try/catch (Exception)` in a suspend function, add a leading
+`catch (e: CancellationException) { throw e }` — this check covers `runCatching` only.
+
+Scope is the **nearest enclosing named function** being `suspend` (matching detekt's
+`SuspendFunSwallowedCancellation`), detected via the `Continuation` parameter on its light method — so
+`async { runCatching { … } }` inside a suspend function is caught (lambdas are transparent), while a
+`runCatching` in a coroutine-builder lambda inside a *non-suspend* function is deliberately out of scope
+(the boundary isn't visible without heuristics; guard those by hand). A `runCatching` around purely
+non-suspending work in a suspend function is still flagged — the wrapper is a strict superset, so it's
+never wrong, and the rule stays a simple ban rather than a fragile "does this lambda suspend" analysis.
+
 ## Lifecycle
 
 Each check names its reason to exist and its reason to die. Regenerate the baseline after landing a
@@ -92,6 +115,7 @@ check, then only ever shrink it.
 | `UnwrappedClockValue` | foreign clock namespace, coming to rest   | never (can't forbid `Long`)                           |
 | `PrematureUnwrap`     | the elimination door of the typed region  | a real module boundary makes `.epochMs` cross-module  |
 | `WireTimeEscape`      | adapter-only consumption of wire DTOs     | `:api` becomes a Gradle module with `internal` DTOs   |
+| `SwallowedCancellation` | `runCatching` swallowing cancellation in suspend code | Kotlin/lint ships an equivalent built-in check |
 
 **Enrollment sweep — nothing left to enroll.** The plan anticipated adding the legacy Jackson
 `io/elements` getters (`ObaArrivalInfo#getScheduledArrivalTime`, …) to `CLOCK_SOURCES`. That surface has
@@ -105,7 +129,7 @@ CLAUDE.md. The checks are a latch discipline, not a suggestion.
 ## Status: enforced
 
 Wired into the app via `lintChecks(project(":lint-rules"))` in `onebusaway-android/build.gradle.kts`, so a
-**new** violation of any of the four issues fails the build under the strict `-PwarningsAsErrors` gate
+**new** violation of any of the five issues fails the build under the strict `-PwarningsAsErrors` gate
 (verified). There is **no lint baseline** — the app is kept clean under the full catalog, so every
 sanctioned pre-existing site is handled *at the site*, not grandfathered in a file:
 
@@ -123,8 +147,11 @@ sanctioned pre-existing site is handled *at the site*, not grandfathered in a fi
   visible at the site, not hidden in this file. Keep it empty.
 - **0** `WireTimeEscape` — the migration left no wire-field read in app logic; the check starts empty and
   exists to keep it that way.
+- **0** `SwallowedCancellation` — every bare `runCatching` in a suspend function was migrated to
+  `runCatchingCancellable` (#1908 / #1921 audit); the check starts empty and keeps new coroutine code on
+  the sanctioned wrapper.
 
-Keep all four checks at zero: mint each new site (or inline-`@Suppress` a genuinely-sanctioned one with a
+Keep all five checks at zero: mint each new site (or inline-`@Suppress` a genuinely-sanctioned one with a
 rationale) — don't reintroduce a baseline.
 
 ## Develop

lint-rules/build.gradle.kts

@@ -35,6 +35,6 @@ dependencies {
 // Lint discovers the registry through this manifest attribute when the jar is loaded via lintChecks.
 tasks.jar {
     manifest {
-        attributes(mapOf("Lint-Registry-v2" to "org.onebusaway.lint.TimeDomainIssueRegistry"))
+        attributes(mapOf("Lint-Registry-v2" to "org.onebusaway.lint.OneBusAwayIssueRegistry"))
     }
 }

lint-rules/src/main/java/org/onebusaway/lint/TimeDomainIssueRegistry.kt → lint-rules/src/main/java/org/onebusaway/lint/OneBusAwayIssueRegistry.kt

@@ -24,14 +24,15 @@ import com.android.tools.lint.detector.api.Issue
  * Registry that publishes onebusaway-android's custom lint checks. Discovered via the
  * `Lint-Registry-v2` manifest attribute set in this module's `build.gradle`.
  */
-class TimeDomainIssueRegistry : IssueRegistry() {
+class OneBusAwayIssueRegistry : IssueRegistry() {
 
     override val issues: List<Issue> =
         listOf(
             RawTimeDetector.ISSUE,
             RawTimeDetector.ISSUE_VALUE,
             PrematureUnwrapDetector.ISSUE,
             WireTimeEscapeDetector.ISSUE,
+            SwallowedCancellationDetector.ISSUE,
         )
 
     override val api: Int = CURRENT_API

lint-rules/src/main/java/org/onebusaway/lint/SwallowedCancellationDetector.kt

@@ -0,0 +1,119 @@
+/*
+ * Copyright (C) 2026 Open Transit Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onebusaway.lint
+
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Implementation
+import com.android.tools.lint.detector.api.Issue
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.Scope
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import com.intellij.psi.PsiMethod
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.getParentOfType
+
+/**
+ * Gives the app a phobia of bare `runCatching` in coroutine code. `kotlin.runCatching` catches every
+ * `Throwable`, so inside a `suspend` function it also swallows the `CancellationException` a cancelled
+ * coroutine throws from a suspend call — turning cancellation into an ordinary `Result.failure`. The
+ * cancelled coroutine then keeps running and its callers read the cancellation as a normal error/empty
+ * state; structured concurrency breaks. That is the #1908/#1921 bug class, and it's invisible on the
+ * happy path and in review.
+ *
+ * `CancellationException` is a plain `IllegalStateException` subtype, so there is no type-level guard —
+ * the compiler can't tell a swallow from a legitimate catch. So this detector keys on the shape: a
+ * `runCatching` whose **nearest enclosing named function is `suspend`** (the coroutine boundary lint can
+ * see deterministically). The sanctioned replacement is `runCatchingCancellable` (in
+ * `org.onebusaway.android.util`), which is behaviour-identical but rethrows `CancellationException`.
+ *
+ * Scope is the enclosing `suspend` function, matching detekt's `SuspendFunSwallowedCancellation`. A
+ * `runCatching` in a coroutine-builder lambda inside a *non-suspend* function (e.g. `scope.launch { … }`)
+ * is deliberately out of scope — the enclosing named function isn't `suspend`, so lint can't see the
+ * boundary without heuristics; guard those by hand.
+ */
+class SwallowedCancellationDetector : Detector(), SourceCodeScanner {
+
+    override fun getApplicableMethodNames(): List<String> = listOf("runCatching")
+
+    override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
+        // Both the receiverless `runCatching` and the `T.runCatching` extension live in kotlin.ResultKt;
+        // confirm the owner so a same-named function elsewhere doesn't match.
+        if (method.containingClass?.qualifiedName != KOTLIN_RESULT_KT) return
+        // Only inside a suspend function — that's where a CancellationException can be thrown and where
+        // swallowing it breaks cancellation. Lambdas are skipped, so `async { runCatching { … } }` in a
+        // suspend function is still attributed to that suspend function.
+        val enclosing = node.getParentOfType<UMethod>() ?: return
+        if (!enclosing.isSuspendFunction()) return
+        context.report(
+            ISSUE,
+            node,
+            context.getLocation(node),
+            "`runCatching` in a `suspend` function also catches `CancellationException`, converting a " +
+                "cancelled coroutine into a `Result.failure` (the cancellation stops propagating and the " +
+                "work keeps running). Use `runCatchingCancellable` " +
+                "(org.onebusaway.android.util), which rethrows cancellation and is otherwise identical.",
+        )
+    }
+
+    /**
+     * True when this function carries the `suspend` modifier. A Kotlin `suspend fun` compiles with a
+     * trailing `kotlin.coroutines.Continuation` parameter on its light (`javaPsi`) method, which is how
+     * lint sees suspension without depending on Kotlin compiler PSI.
+     */
+    private fun UMethod.isSuspendFunction(): Boolean =
+        javaPsi.parameterList.parameters.lastOrNull()
+            ?.type?.canonicalText?.startsWith(CONTINUATION_FQN) == true
+
+    companion object {
+        private const val KOTLIN_RESULT_KT = "kotlin.ResultKt"
+        private const val CONTINUATION_FQN = "kotlin.coroutines.Continuation"
+
+        @JvmField
+        val ISSUE: Issue = Issue.create(
+            id = "SwallowedCancellation",
+            briefDescription = "runCatching in a suspend function swallows CancellationException",
+            explanation = """
+                `kotlin.runCatching` catches every `Throwable`. Inside a `suspend` function that includes \
+                the `CancellationException` a cancelled coroutine throws from a suspend call, so the \
+                cancellation is captured into the returned `Result` instead of propagating. The cancelled \
+                coroutine keeps running, structured concurrency breaks, and callers see the cancellation \
+                as an ordinary failure/empty result. `CancellationException` is a plain \
+                `IllegalStateException` subtype, so nothing at the type level catches this — it passes \
+                compilation and review, and only misbehaves under real cancellation (screen closed, \
+                search superseded, poll tick outrun). This is the #1908 / #1921 bug class.
+
+                Replace it with `runCatchingCancellable` from `org.onebusaway.android.util`: it is \
+                behaviour-identical to `runCatching` but rethrows `CancellationException` (a real failure \
+                still resolves to `Result.failure`). For a broad `try/catch (Exception)` in a suspend \
+                function, add a leading `catch (e: CancellationException) { throw e }` instead — this \
+                check covers `runCatching` only.
+
+                If a `suspend` function genuinely must capture cancellation (extremely rare), suppress \
+                this id at the site with a one-line rationale and a tracking issue.
+            """,
+            category = Category.CORRECTNESS,
+            priority = 6,
+            severity = Severity.WARNING,
+            implementation = Implementation(
+                SwallowedCancellationDetector::class.java,
+                Scope.JAVA_FILE_SCOPE,
+            ),
+        )
+    }
+}

lint-rules/src/test/java/org/onebusaway/lint/SwallowedCancellationDetectorTest.kt

@@ -0,0 +1,106 @@
+/*
+ * Copyright (C) 2026 Open Transit Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onebusaway.lint
+
+import com.android.tools.lint.checks.infrastructure.TestFiles.kotlin
+import com.android.tools.lint.checks.infrastructure.TestLintTask.lint
+import org.junit.Test
+
+class SwallowedCancellationDetectorTest {
+
+    private fun lintKotlin(vararg sources: String) =
+        lint()
+            .files(*sources.map { kotlin(it).indented() }.toTypedArray())
+            .issues(SwallowedCancellationDetector.ISSUE)
+            .allowMissingSdk()
+            .run()
+
+    /** The motivating case: bare `runCatching` directly in a suspend function body. */
+    @Test
+    fun flagsRunCatchingInSuspendFunction() {
+        lintKotlin(
+            """
+            package test
+            suspend fun load(): Result<Int> = runCatching { fetch() }
+            suspend fun fetch(): Int = 1
+            """,
+        ).expectWarningCount(1)
+    }
+
+    /** Nested in an `async { … }` lambda, but the enclosing named function is still suspend — flagged. */
+    @Test
+    fun flagsRunCatchingInLambdaInsideSuspendFunction() {
+        lintKotlin(
+            """
+            package test
+            suspend fun load(): Result<Int> {
+                lateinit var r: Result<Int>
+                run { r = runCatching { fetch() } }
+                return r
+            }
+            suspend fun fetch(): Int = 1
+            """,
+        ).expectWarningCount(1)
+    }
+
+    /** The sanctioned wrapper must not be flagged (different symbol). */
+    @Test
+    fun doesNotFlagRunCatchingCancellable() {
+        lintKotlin(
+            """
+            package test
+            fun <T> runCatchingCancellable(block: () -> T): Result<T> = runCatching(block)
+            suspend fun load(): Result<Int> = runCatchingCancellable { fetch() }
+            suspend fun fetch(): Int = 1
+            """,
+        ).expectClean()
+    }
+
+    /** `runCatching` in a plain (non-suspend) function is fine — no coroutine, no cancellation. */
+    @Test
+    fun doesNotFlagRunCatchingInNonSuspendFunction() {
+        lintKotlin(
+            """
+            package test
+            fun parse(s: String): Result<Int> = runCatching { s.toInt() }
+            """,
+        ).expectClean()
+    }
+
+    /** The wrapper's own body: `runCatching` inside a non-suspend inline fun is not flagged. */
+    @Test
+    fun doesNotFlagInsideNonSuspendInlineWrapper() {
+        lintKotlin(
+            """
+            package test
+            inline fun <T> guard(block: () -> T): Result<T> =
+                runCatching(block).onFailure { }
+            """,
+        ).expectClean()
+    }
+
+    /** A same-named `runCatching` that is not kotlin.runCatching must not be flagged. */
+    @Test
+    fun doesNotFlagUnrelatedRunCatchingFunction() {
+        lintKotlin(
+            """
+            package test
+            class Helper { fun runCatching(block: () -> Int): Int = block() }
+            suspend fun load(h: Helper): Int = h.runCatching { 1 }
+            """,
+        ).expectClean()
+    }
+}

onebusaway-android/src/main/java/org/onebusaway/android/api/data/StopsForRouteRepository.kt

@@ -18,7 +18,6 @@ package org.onebusaway.android.api.data
 import android.location.Location
 import javax.inject.Inject
 import javax.inject.Singleton
-import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
@@ -38,6 +37,7 @@ import org.onebusaway.android.models.RouteMapStop
 import org.onebusaway.android.models.RouteStopGroup
 import org.onebusaway.android.util.PolylineDecoder
 import org.onebusaway.android.util.SingleFlight
+import org.onebusaway.android.util.runCatchingCancellable
 import java.io.IOException
 
 /**
@@ -111,10 +111,9 @@ class DefaultStopsForRouteRepository internal constructor(
         val fetched = entry(routeId).getOrElse { return Result.failure(it) }
             ?: return Result.success(null) // no endpoint — nothing to show
         // Decoding the route's shape polylines is the one bit of non-trivial CPU work in this layer.
-        // runCatching swallows everything, so rethrow a CancellationException (raised by withContext if
-        // the caller is cancelled mid-decode) rather than reporting the abandoned work as a failure.
-        return runCatching { withContext(Dispatchers.Default) { fetched.toRouteMapData(routeId) } }
-            .onFailure { if (it is CancellationException) throw it }
+        // runCatchingCancellable rethrows a CancellationException (raised by withContext if the caller is
+        // cancelled mid-decode) rather than reporting the abandoned work as a failure.
+        return runCatchingCancellable { withContext(Dispatchers.Default) { fetched.toRouteMapData(routeId) } }
     }
 
     /**