Skip to content

Commit bdca56e

Browse files
ADFA-5067: Stop a link dying permanently on a failed or unverifiable resolve
Two ways a deep link could go silently dead forever, both in the consumption bookkeeping. A transient filesystem failure was recorded as "no such project". resolveWithinDirectory maps everything that is not Contained to null, so ContainedPathResolver's Resolution.Unverifiable -- an IOException the resolver models explicitly as "not an escape, refused because unproven" -- arrived at findValidProjectByName indistinguishable from a genuine miss. An EACCES right after a storage-permission change, or an EIO on a flaky SD/FUSE mount, therefore told the user "No project named X was found" about a project that plainly exists, and MainActivity then recorded the request consumed on the stated reasoning that the project does not exist -- so the identical URL was a silent no-op on every later delivery. The SecurityException path had the same problem. Add lookupValidProjectByName, returning Found/NotFound/Unverifiable, and have resolveDeepLinkProject return the matching tri-state instead of File?. Only a definitive NotFound is recorded consumed. Unverifiable now reports the scan-failed message rather than "no project named X": telling someone a project they can see in the projects list does not exist is worse than admitting the lookup failed. An Unverifiable from one Unicode normal form does not mask a Found from another -- it is remembered and only returned if no candidate form resolves. findValidProjectByName stays, reduced to a null-or-directory view for the callers that cannot act on the difference. A fresh tap was mistaken for a programmatic re-delivery. The re-forward gate dropped any request already in consumedDeepLinkRequests. It exists to stop a bounce loop -- MainActivity opens a project, the editor decides the link names a different one and bounces it back, and the dialog goes straight back up -- but it could not tell that loop from a genuinely new tap that happens to be re-forwarded, and DeepLinkRequest carries no nonce, so a repeat tap is equal by value to the earlier one. Tapping a link for a project that does not exist yet, creating it, then tapping again was dropped with no dialog, no error and no log, on that and every subsequent tap -- which is the flow the feature exists for. Track the failed-resolve consumptions separately in unresolvedDeepLinkRequests and exempt them from the gate. A request that never resolved never reached the editor, so no bounce can originate from it and the loop cannot come back. The set is persisted alongside consumedDeepLinkRequests, or the same sequence across a process death lands in the identical hole, and a request is dropped from it as soon as it is retried, so a later success stops the exemption. The Unverifiable branch has no automated coverage: provoking a real EACCES/EIO from the filesystem mid-call is not something a JVM unit test can do reliably. The new tests pin the two outcomes that are reachable, plus that findValidProjectByName still agrees with the lookup it now delegates to. Claude-Session: https://claude.ai/code/session_01LuyR4ajssmKw1o1UQsjJbP
1 parent e8bd904 commit bdca56e

5 files changed

Lines changed: 207 additions & 33 deletions

File tree

app/src/main/java/com/itsaky/androidide/activities/MainActivity.kt

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import com.itsaky.androidide.shortcuts.ShortcutContext
6161
import com.itsaky.androidide.shortcuts.ShortcutExecutionContext
6262
import com.itsaky.androidide.shortcuts.ShortcutManager
6363
import com.itsaky.androidide.templates.ITemplateProvider
64+
import com.itsaky.androidide.utils.DeepLinkProjectLookup
6465
import com.itsaky.androidide.utils.DialogUtils
6566
import com.itsaky.androidide.utils.Environment
6667
import com.itsaky.androidide.utils.FeatureFlags
@@ -120,6 +121,22 @@ class MainActivity : EdgeToEdgeIDEActivity() {
120121
// was not enough.
121122
private val consumedDeepLinkRequests = ConsumedRequests<DeepLinkRequest>()
122123

124+
// The subset of consumedDeepLinkRequests recorded because the project could not be resolved,
125+
// rather than because anything was actually opened.
126+
//
127+
// The two have to be told apart by onNewIntent's re-forward gate. That gate exists to stop a
128+
// bounce loop: this activity opens a project, the editor decides the link names a different one
129+
// and bounces it straight back, and without the gate its dialog goes right back up. But a request
130+
// that failed to resolve never reached the editor at all, so no bounce can originate from it --
131+
// and DeepLinkRequest carries no nonce, so a genuinely new tap of the same URL is equal by value
132+
// to the failed one. Tapping a link for a project that does not exist yet, creating it, and
133+
// tapping again therefore died silently, forever, which is exactly the teacher-sends-a-student-a-
134+
// link flow the feature is for (ADFA-5067 review).
135+
//
136+
// Persisted alongside consumedDeepLinkRequests: without that, the same tap-create-tap sequence
137+
// across a process death lands back in the identical hole.
138+
private val unresolvedDeepLinkRequests = ConsumedRequests<DeepLinkRequest>()
139+
123140
private val onBackPressedCallback =
124141
object : OnBackPressedCallback(true) {
125142
override fun handleOnBackPressed() {
@@ -161,6 +178,11 @@ class MainActivity : EdgeToEdgeIDEActivity() {
161178
BundleCompat.getParcelableArrayList(it, KEY_CONSUMED_DEEP_LINK_REQUESTS, DeepLinkRequest::class.java)
162179
},
163180
)
181+
unresolvedDeepLinkRequests.restore(
182+
savedInstanceState?.let {
183+
BundleCompat.getParcelableArrayList(it, KEY_UNRESOLVED_DEEP_LINK_REQUESTS, DeepLinkRequest::class.java)
184+
},
185+
)
164186
// A config change this activity doesn't declare (e.g. font scale, day/night) recreates it with
165187
// savedInstanceState != null while handleDeepLinkRequest's resolve may still be in flight --
166188
// the old instance's lifecycleScope (and its coroutine) is cancelled with it. Gating solely on
@@ -604,7 +626,11 @@ class MainActivity : EdgeToEdgeIDEActivity() {
604626
// -- forever, for links naming a project that did not exist when first tapped.
605627
?.takeIf {
606628
!intent.getBooleanExtra(EditorIntentExtras.EXTRA_REFORWARDED_DEEP_LINK, false) ||
607-
it !in consumedDeepLinkRequests
629+
it !in consumedDeepLinkRequests ||
630+
// A request consumed only because its project could not be resolved never reached
631+
// the editor, so this bounce cannot be the loop the gate guards against -- it is a
632+
// fresh tap that happens to be value-equal to the earlier failure. Let it retry.
633+
it in unresolvedDeepLinkRequests
608634
}?.let { handleDeepLinkRequest(it) }
609635
}
610636

@@ -624,25 +650,38 @@ class MainActivity : EdgeToEdgeIDEActivity() {
624650
* project open with no user interaction at all. `DeepLinkTargetsNotExportedTest` pins that.
625651
*/
626652
private fun handleDeepLinkRequest(request: DeepLinkRequest) {
653+
// This attempt supersedes any earlier unresolved one for the same request. If it fails to
654+
// resolve again the failure branch re-records it; if it succeeds the request must stop being
655+
// exempt from the re-forward gate, or the bounce loop that gate exists to stop could resume.
656+
unresolvedDeepLinkRequests.remove(request)
627657
latestDeepLinkRequest = request
628658
lifecycleScope.launch(Dispatchers.IO) {
629-
val projectDir = resolveDeepLinkProject(projectsRoot(), request.projectName)
659+
val lookup = resolveDeepLinkProject(projectsRoot(), request.projectName)
630660
withContext(Dispatchers.Main) {
631661
// The activity may have started finishing while resolveDeepLinkProject was still
632662
// scanning disk -- lifecycleScope only cancels at ON_DESTROY, not the moment isFinishing
633663
// first flips true, so this continuation can otherwise still run and show a dialog on a
634664
// dying window.
635665
if (isFinishing || isDestroyed) return@withContext
636-
if (projectDir == null) {
666+
if (lookup !is DeepLinkProjectLookup.Found) {
637667
// Consumed even though nothing opened: the project does not exist, so retrying on
638668
// every recreate only re-shows "No project named X was found" indefinitely. Recorded
639669
// only when this request is still the current one, so a superseded slow resolve
640670
// cannot consume the newer request's slot.
641-
if (latestDeepLinkRequest === request) {
671+
//
672+
// NotFound only. An Unverifiable result -- an EACCES straight after a
673+
// storage-permission change, an EIO on a flaky SD/FUSE mount -- says nothing about
674+
// whether the project exists, and recording it made a momentary filesystem failure
675+
// silence a valid link on every later delivery (ADFA-5067 review).
676+
if (lookup is DeepLinkProjectLookup.NotFound && latestDeepLinkRequest === request) {
642677
consumedDeepLinkRequests.add(request)
678+
// Tracked apart from the general consumed set so the re-forward gate in
679+
// onNewIntent can let this request through again -- see the field's docs.
680+
unresolvedDeepLinkRequests.add(request)
643681
}
644682
return@withContext
645683
}
684+
val projectDir = lookup.projectDir
646685
// A second, faster-resolving deep link superseded this one while it was still resolving
647686
// -- this stale, slower request must not now bounce the user back to its own (older)
648687
// target after they've already been taken to the newer one.
@@ -672,9 +711,11 @@ class MainActivity : EdgeToEdgeIDEActivity() {
672711
override fun onSaveInstanceState(outState: Bundle) {
673712
super.onSaveInstanceState(outState)
674713
outState.putParcelableArrayList(KEY_CONSUMED_DEEP_LINK_REQUESTS, consumedDeepLinkRequests.toSavedList())
714+
outState.putParcelableArrayList(KEY_UNRESOLVED_DEEP_LINK_REQUESTS, unresolvedDeepLinkRequests.toSavedList())
675715
}
676716

677717
companion object {
678718
private const val KEY_CONSUMED_DEEP_LINK_REQUESTS = "consumedDeepLinkRequests"
719+
private const val KEY_UNRESOLVED_DEEP_LINK_REQUESTS = "unresolvedDeepLinkRequests"
679720
}
680721
}

app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ import com.itsaky.androidide.tasks.executeAsync
108108
import com.itsaky.androidide.tooling.api.messages.result.TaskExecutionResult
109109
import com.itsaky.androidide.ui.ARCHIVE_EXTENSIONS
110110
import com.itsaky.androidide.ui.CodeEditorView
111+
import com.itsaky.androidide.utils.DeepLinkProjectLookup
111112
import com.itsaky.androidide.utils.DialogUtils.newMaterialDialogBuilder
112113
import com.itsaky.androidide.utils.DialogUtils.showConfirmationDialog
113114
import com.itsaky.androidide.utils.EditorActivityActions
@@ -2737,8 +2738,8 @@ open class EditorHandlerActivity :
27372738
latestDeepLinkRequest = request
27382739

27392740
lifecycleScope.launch(Dispatchers.IO) {
2740-
val projectDir = resolveDeepLinkProject(projectsRoot(), request.projectName)
2741-
if (projectDir == null) {
2741+
val lookup = resolveDeepLinkProject(projectsRoot(), request.projectName)
2742+
if (lookup !is DeepLinkProjectLookup.Found) {
27422743
// No such project, so the switch this intent announced is never going to happen. Without
27432744
// this the capture above is stranded: setIntent() has already dropped the staying
27442745
// project's pending file request, nothing puts it back, and
@@ -2776,7 +2777,7 @@ open class EditorHandlerActivity :
27762777
// A newer deep link's onNewIntent call already superseded this one -- switching to
27772778
// this stale target now would undo the newer request the user actually tapped.
27782779
if (latestDeepLinkRequest !== request) return@withContext
2779-
switchToProject(projectDir.absolutePath, request.fileRequest)
2780+
switchToProject(lookup.projectDir.absolutePath, request.fileRequest)
27802781
}
27812782
}
27822783
}

app/src/main/java/com/itsaky/androidide/utils/DeepLinkProjectResolution.kt

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,40 +27,74 @@ import java.io.File
2727

2828
private val log = LoggerFactory.getLogger("DeepLinkProjectResolution")
2929

30+
/**
31+
* The outcome of [resolveDeepLinkProject]. Keeps "no such project" apart from "could not tell",
32+
* which a bare `File?` collapsed: callers record a definitive absence so the link stops re-reporting
33+
* itself on every recreate, and recording an [Unverifiable] the same way made a momentary
34+
* filesystem failure kill a perfectly valid link permanently (ADFA-5067 review).
35+
*/
36+
sealed interface DeepLinkProjectLookup {
37+
data class Found(
38+
val projectDir: File,
39+
) : DeepLinkProjectLookup
40+
41+
/** No project of that name exists. Definitive, so callers may remember it. */
42+
data object NotFound : DeepLinkProjectLookup
43+
44+
/** The lookup failed for a reason unrelated to the project's existence. Remember nothing. */
45+
data object Unverifiable : DeepLinkProjectLookup
46+
}
47+
3048
/**
3149
* Resolves [projectName] to a validated project directory under [projectsRoot] for a deep link,
32-
* handling the [SecurityException] [findValidProjectByName] can throw and reporting both "not
33-
* found" and "scan failed" to the user via `flashError` on the main thread. A `null` result means
34-
* the caller can just return -- either failure case already flashed its own message.
50+
* reporting every failure to the user via `flashError` on the main thread. The caller can return on
51+
* anything but [DeepLinkProjectLookup.Found] -- each failure case has already shown its own message
52+
* -- but must consult *which* failure it was before recording the request as dealt with.
3553
*
3654
* Call from a background dispatcher (e.g. `Dispatchers.IO`); this only switches to
3755
* [Dispatchers.Main] itself for the user-facing error messages.
3856
*/
3957
suspend fun Activity.resolveDeepLinkProject(
4058
projectsRoot: File,
4159
projectName: String,
42-
): File? {
43-
val projectDir =
60+
): DeepLinkProjectLookup {
61+
val lookup =
4462
try {
45-
findValidProjectByName(projectsRoot, projectName)
63+
lookupValidProjectByName(projectsRoot, projectName)
4664
} catch (e: CancellationException) {
4765
throw e
4866
} catch (e: SecurityException) {
4967
log.error("Failed to scan {} for deep link", projectsRoot, e)
50-
withContext(Dispatchers.Main) {
51-
// Re-checked here, not before the hop -- the activity can start finishing during the
52-
// hop itself, and a check taken only beforehand would miss that window.
53-
if (!isFinishing && !isDestroyed) flashError(getString(string.msg_deeplink_scan_failed))
54-
}
55-
return null
68+
flashOnMain(getString(string.msg_deeplink_scan_failed))
69+
// A denied scan says nothing about whether the project is there.
70+
return DeepLinkProjectLookup.Unverifiable
5671
}
5772

58-
if (projectDir == null) {
59-
withContext(Dispatchers.Main) {
60-
if (!isFinishing && !isDestroyed) {
61-
flashError(getString(string.msg_deeplink_project_not_found, projectName))
62-
}
73+
return when (lookup) {
74+
is ProjectNameLookup.Found -> {
75+
DeepLinkProjectLookup.Found(lookup.dir)
6376
}
77+
78+
is ProjectNameLookup.Unverifiable -> {
79+
log.error("Could not determine whether project {} exists under {}", projectName, projectsRoot, lookup.cause)
80+
// Deliberately the scan-failed message, not "no project named X": telling the user that a
81+
// project they can see in the projects list does not exist is worse than saying the
82+
// lookup failed.
83+
flashOnMain(getString(string.msg_deeplink_scan_failed))
84+
DeepLinkProjectLookup.Unverifiable
85+
}
86+
87+
ProjectNameLookup.NotFound -> {
88+
flashOnMain(getString(string.msg_deeplink_project_not_found, projectName))
89+
DeepLinkProjectLookup.NotFound
90+
}
91+
}
92+
}
93+
94+
private suspend fun Activity.flashOnMain(message: String) {
95+
withContext(Dispatchers.Main) {
96+
// Re-checked here, not before the hop -- the activity can start finishing during the hop
97+
// itself, and a check taken only beforehand would miss that window.
98+
if (!isFinishing && !isDestroyed) flashError(message)
6499
}
65-
return projectDir
66100
}

app/src/main/java/com/itsaky/androidide/utils/ProjectValidations.kt

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.itsaky.androidide.utils
22

33
import java.io.File
4+
import java.io.IOException
45
import java.text.Normalizer
56
import kotlin.collections.filter
67
import kotlin.collections.orEmpty
@@ -22,6 +23,32 @@ internal fun findValidProjects(projectsRoot: File): List<File> {
2223
return subdirs.filter { dir -> isValidProjectDirectory(dir) }
2324
}
2425

26+
/**
27+
* The outcome of [lookupValidProjectByName], which unlike a bare `File?` keeps "no project by that
28+
* name" apart from "whether one exists could not be determined".
29+
*
30+
* Callers act on the two very differently: a definitive absence is worth remembering (so a link
31+
* naming a project that does not exist stops re-reporting itself on every recreate), while an
32+
* unverifiable result says nothing at all about the project and must leave every such decision
33+
* untouched.
34+
*/
35+
internal sealed interface ProjectNameLookup {
36+
data class Found(
37+
val dir: File,
38+
) : ProjectNameLookup
39+
40+
/** No project of that name exists under the projects root. */
41+
data object NotFound : ProjectNameLookup
42+
43+
/**
44+
* A filesystem failure other than absence (EACCES right after a storage-permission change, EIO
45+
* on a flaky SD/FUSE mount) stopped the lookup from reaching an answer.
46+
*/
47+
data class Unverifiable(
48+
val cause: IOException,
49+
) : ProjectNameLookup
50+
}
51+
2552
/**
2653
* Resolves [name] directly to `[projectsRoot]/[name]` and validates just that one directory --
2754
* the O(1) counterpart to [findValidProjects] for callers (e.g. deep links) that already know the
@@ -31,36 +58,62 @@ internal fun findValidProjects(projectsRoot: File): List<File> {
3158
* [resolveWithinDirectory] rather than a bare `File(projectsRoot, name)` -- [findValidProjects]
3259
* only ever matches against names of directories it already enumerated under [projectsRoot], so it
3360
* can't be pointed outside it, but a direct `File(root, name)` join can (e.g. `name = "../../etc"`).
61+
*
62+
* Reports *why* it found nothing -- see [ProjectNameLookup]; [findValidProjectByName] is this
63+
* reduced to a nullable directory for callers that cannot act on the difference.
3464
*/
35-
internal fun findValidProjectByName(
65+
internal fun lookupValidProjectByName(
3666
projectsRoot: File,
3767
name: String,
38-
): File? {
68+
): ProjectNameLookup {
3969
// A project name is always a single path segment. resolveWithinDirectory's lexical check only
4070
// rejects ".."/a leading separator, so without this, name = "." would resolve to projectsRoot
4171
// itself (opening the whole projects directory as "a project" if it happens to satisfy
4272
// isValidProjectDirectory), and an embedded separator like "foo/bar" would resolve two levels
4373
// deep instead of naming a direct child.
4474
if (name.isEmpty() || name == "." || name.contains("/") || name.contains("\\")) {
45-
return null
75+
return ProjectNameLookup.NotFound
4676
}
47-
if (!projectsRoot.isProjectCandidateDir()) return null
77+
if (!projectsRoot.isProjectCandidateDir()) return ProjectNameLookup.NotFound
4878

4979
// A deep-link name is typically authored/normalized as NFC by web tooling, but an on-disk
5080
// project directory imported from elsewhere (e.g. a git clone authored on macOS, which
5181
// decomposes accented filenames to NFD) may not codepoint-match it even though the two look
5282
// identical. Try both normal forms -- still O(1) filesystem lookups, not a directory scan --
5383
// rather than reporting a visually-identical project as "not found".
5484
val candidateNames = linkedSetOf(name, Normalizer.normalize(name, Normalizer.Form.NFC), Normalizer.normalize(name, Normalizer.Form.NFD))
85+
// Remembered rather than returned on the spot: a later candidate form may still resolve cleanly,
86+
// and a definite Found has to win over an earlier form's transient IO failure.
87+
var unverifiable: IOException? = null
88+
val resolver = ContainedPathResolver(projectsRoot)
5589
for (candidateName in candidateNames) {
56-
val candidate = resolveWithinDirectory(projectsRoot, candidateName) ?: continue
57-
if (candidate.isProjectCandidateDir() && isValidProjectDirectory(candidate)) {
58-
return candidate
90+
when (val resolution = resolver.resolve(candidateName)) {
91+
is ContainedPathResolver.Resolution.Contained -> {
92+
val candidate = resolution.file
93+
if (candidate.isProjectCandidateDir() && isValidProjectDirectory(candidate)) {
94+
return ProjectNameLookup.Found(candidate)
95+
}
96+
}
97+
98+
is ContainedPathResolver.Resolution.Unverifiable -> {
99+
unverifiable = resolution.cause
100+
}
101+
102+
// A traversal attempt is a definitive "not this project", not an unknown.
103+
is ContainedPathResolver.Resolution.Rejected -> {
104+
Unit
105+
}
59106
}
60107
}
61-
return null
108+
return unverifiable?.let(ProjectNameLookup::Unverifiable) ?: ProjectNameLookup.NotFound
62109
}
63110

111+
/** [lookupValidProjectByName] reduced to the project directory, or null for any other outcome. */
112+
internal fun findValidProjectByName(
113+
projectsRoot: File,
114+
name: String,
115+
): File? = (lookupValidProjectByName(projectsRoot, name) as? ProjectNameLookup.Found)?.dir
116+
64117
/**
65118
* True if [a] and [b] name the same project, tolerating an NFC/NFD codepoint difference (e.g. an
66119
* accented project name authored as NFD on macOS vs. the NFC form a deep-link URL typically

app/src/test/java/com/itsaky/androidide/utils/ProjectValidationsTest.kt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,4 +102,49 @@ class ProjectValidationsTest {
102102

103103
assertThat(findValidProjectByName(root, "..")).isNull()
104104
}
105+
106+
// The tri-state lookup findValidProjectByName now delegates to (ADFA-5067 review). These pin the
107+
// two outcomes a unit test can actually produce; Unverifiable needs a real EACCES/EIO from the
108+
// filesystem mid-call, which is not reliably provokable in a JVM test -- see the class docs.
109+
@Test
110+
fun `lookup reports Found for an existing project`() {
111+
val root = tempFolder.newFolder("projects")
112+
val project = makeValidProject(root, "MyApp")
113+
114+
val lookup = lookupValidProjectByName(root, "MyApp")
115+
116+
assertThat(lookup).isInstanceOf(ProjectNameLookup.Found::class.java)
117+
assertThat((lookup as ProjectNameLookup.Found).dir.canonicalFile).isEqualTo(project.canonicalFile)
118+
}
119+
120+
@Test
121+
fun `lookup reports NotFound for a name with no project`() {
122+
val root = tempFolder.newFolder("projects")
123+
124+
assertThat(lookupValidProjectByName(root, "DoesNotExist")).isEqualTo(ProjectNameLookup.NotFound)
125+
}
126+
127+
// A traversal attempt is a definite "not this project", not an unknown -- callers are allowed to
128+
// remember a NotFound, and must not be handed something they have to treat as maybe-transient.
129+
@Test
130+
fun `lookup reports NotFound for a traversal attempt`() {
131+
val root = tempFolder.newFolder("projects")
132+
133+
assertThat(lookupValidProjectByName(root, "../etc")).isEqualTo(ProjectNameLookup.NotFound)
134+
assertThat(lookupValidProjectByName(root, ".")).isEqualTo(ProjectNameLookup.NotFound)
135+
assertThat(lookupValidProjectByName(root, "")).isEqualTo(ProjectNameLookup.NotFound)
136+
}
137+
138+
// findValidProjectByName is now a thin reduction of lookupValidProjectByName; this pins that the
139+
// refactor did not change what the many existing callers see.
140+
@Test
141+
fun `findValidProjectByName still agrees with the lookup it delegates to`() {
142+
val root = tempFolder.newFolder("projects")
143+
makeValidProject(root, "MyApp")
144+
145+
for (name in listOf("MyApp", "DoesNotExist", "../etc", ".", "")) {
146+
val expected = (lookupValidProjectByName(root, name) as? ProjectNameLookup.Found)?.dir
147+
assertThat(findValidProjectByName(root, name)).isEqualTo(expected)
148+
}
149+
}
105150
}

0 commit comments

Comments
 (0)