Import the real "Run only if" condition from Tasker ConditionList

app/src/androidTest/java/com/opentasker/core/engine/variables/VariableExpanderConditionInstrumentedTest.kt

@@ -0,0 +1,75 @@
+package com.opentasker.core.engine.variables
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.opentasker.core.engine.VariableStore
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+
+/**
+ * Runs the Matches/Doesn't Match (Tasker op codes 2/3) condition evaluator against Android's
+ * real ICU-backed regex engine.
+ *
+ * Per CONTRIBUTING.md, this project has shipped desktop-JVM/Android ICU regex divergences three
+ * times already; the JVM unit test suite runs on desktop `java.util.regex` and cannot see that
+ * class of bug by construction. `VariableExpander.matchesGlob` builds its pattern at runtime from
+ * imported Tasker wildcard data rather than from a fixed literal, so it isn't covered by the
+ * generated `production-regex-patterns.txt` corpus either (that scanner finds compile-time regex
+ * literals in source, and this pattern doesn't exist until a condition actually runs). This test
+ * is the only place the real construct -- an `(?s)` inline dotall flag plus `Regex.escape()`
+ * output, wired together by `String.split("*")` -- gets compiled by the engine devices actually
+ * ship.
+ */
+@RunWith(AndroidJUnit4::class)
+class VariableExpanderConditionInstrumentedTest {
+
+    @Test
+    fun matchesWildcardAgainstRealValueOnDevice() {
+        val variables = VariableStore().apply { set("pa_do", "view_url") }
+        assertTrue(variables.evaluateCondition("%pa_do ~ view_url"))
+    }
+
+    @Test
+    fun matchesLeadingAndTrailingWildcardOnDevice() {
+        // The shape actually produced by a real Tasker export for a JSON-substring guard
+        // (verified against a live 6.6.20 backup): op=2, lhs a JSON blob, rhs `*"say":*`.
+        val variables = VariableStore().apply { set("pa_json", "{\"say\":\"hello\"}") }
+        assertTrue(variables.evaluateCondition("%pa_json ~ *\"say\":*"))
+    }
+
+    @Test
+    fun doesNotMatchWildcardOnDevice() {
+        val variables = VariableStore().apply { set("pa_do", "launch_app") }
+        assertFalse(variables.evaluateCondition("%pa_do ~ view_url"))
+    }
+
+    @Test
+    fun notMatchesOperatorNegatesCorrectlyOnDevice() {
+        val variables = VariableStore().apply { set("pa_do", "launch_app") }
+        assertTrue(variables.evaluateCondition("%pa_do !~ view_url"))
+    }
+
+    @Test
+    fun wildcardValueContainingRegexMetacharactersIsTreatedLiterallyOnDevice() {
+        // Everything except "*" must be escaped, not interpreted as regex syntax -- a value or
+        // pattern containing ".", "(", "+", etc. (all realistic in imported %pa_json / %pa_url
+        // condition data) must match literally.
+        val variables = VariableStore().apply { set("pa_url", "https://example.com/a.b+c(d)") }
+        assertTrue(variables.evaluateCondition("%pa_url ~ *example.com/a.b+c(d)*"))
+        assertFalse(variables.evaluateCondition("%pa_url ~ *exampleXcom*"))
+    }
+
+    @Test
+    fun isSetAndNotSetStillEvaluateCorrectlyOnDevice() {
+        // Not regex-related, but cheap to confirm here too since this is the one place a real
+        // ICU-backed VariableStore instance already exists for this evaluator.
+        val unset = VariableStore()
+        assertFalse(unset.evaluateCondition("%text is_set"))
+        assertTrue(unset.evaluateCondition("%text not_set"))
+
+        val set = VariableStore().apply { set("text", "set a timer") }
+        assertTrue(set.evaluateCondition("%text is_set"))
+        assertFalse(set.evaluateCondition("%text not_set"))
+    }
+}

app/src/main/java/com/opentasker/core/engine/variables/VariableExpander.kt

@@ -259,7 +259,18 @@ class VariableExpander {
             return parts.all { evaluateConditionInternal(it, store, arrays) }
         }
 
-        val comparison = parseComparison(normalized) ?: return normalized.toBoolean()
+        // A real binary comparison always wins over the unary is_set/not_set reading: e.g.
+        // "%status == is_set", comparing a variable against the literal string "is_set", must
+        // stay an equality check, not get hijacked into an existence check just because the text
+        // happens to end with that word. Unary existence syntax is tried only once no binary
+        // comparison matches at all -- which also means "is_set"/"not_set" can never appear as a
+        // *value* on either side of a real ==, !=, ~, etc. comparison and be misread as the unary
+        // operator, since parseComparison always gets first look.
+        val comparison = parseComparison(normalized)
+        if (comparison == null) {
+            unaryExistenceResult(normalized, store, arrays)?.let { return it }
+            return normalized.toBoolean()
+        }
         val left = expandInternal(comparison.left, store, arrays)
         val right = expandInternal(comparison.right, store, arrays)
 
@@ -270,6 +281,51 @@ class VariableExpander {
             ComparisonOperator.GE -> compareNumbers(left, right) { l, r -> l >= r }
             ComparisonOperator.LT -> compareNumbers(left, right) { l, r -> l < r }
             ComparisonOperator.GT -> compareNumbers(left, right) { l, r -> l > r }
+            ComparisonOperator.MATCHES -> matchesGlob(left, right)
+            ComparisonOperator.NOT_MATCHES -> !matchesGlob(left, right)
+        }
+    }
+
+    /**
+     * Tasker's wildcard match: `*` is the only special character, everything else is literal.
+     * Used for imported "Matches"/"Doesn't Match" conditions (Tasker op codes 2/3), e.g.
+     * `%pa_do ~ view_url` or `%pa_json ~ *"say":*`. Regex metacharacters other than `*` are
+     * escaped so a pattern like `%pa_x1.example` matches a literal dot, not "any character".
+     */
+    /**
+     * Tasker's wildcard match: `*` is the only special character, everything else is literal.
+     * Used for imported "Matches"/"Doesn't Match" conditions (Tasker op codes 2/3), e.g.
+     * `%pa_do ~ view_url` or `%pa_json ~ *"say":*`.
+     *
+     * Goes through [compileLinearRegex] (RE2, linear-time) with the same [MAX_REGEX_LENGTH] /
+     * [MAX_REGEX_INPUT_LENGTH] guards this file already applies to every other regex built from
+     * import- or variable-derived text (see the `regex:`/`replace:` var-ops above), rather than
+     * Kotlin's backtracking `Regex`: `pattern` here comes from imported Tasker condition data,
+     * not a fixed literal, so several `*` wildcards is exactly the shape that causes catastrophic
+     * backtracking in a standard engine. `com.google.re2j.Pattern.quote` is RE2J's per-character
+     * `quoteMeta` escaper, not `kotlin.text.Regex.escape`'s `\Q...\E` -- RE2 doesn't implement
+     * `\Q...\E` as a syntax construct at all, so the latter would either fail to compile or (worse)
+     * be silently misinterpreted.
+     *
+     * `\A`/`\z` anchor for a full-string match (RE2J's `Matcher` is not confirmed to expose a
+     * `.matches()` convenience the way `java.util.regex.Matcher` does, so this uses the same
+     * `.find()` call already proven out at the other [compileLinearRegex] call sites).
+     */
+    private fun matchesGlob(value: String, pattern: String): Boolean {
+        if (pattern.length > MAX_REGEX_LENGTH || value.length > MAX_REGEX_INPUT_LENGTH) return false
+        val regex = buildString {
+            append("(?s)\\A")
+            pattern.split("*").forEachIndexed { index, literal ->
+                if (index > 0) append(".*")
+                append(Re2Pattern.quote(literal))
+            }
+            append("\\z")
+        }
+        val matcher = compileLinearRegex(regex)?.matcher(value) ?: return false
+        return try {
+            matcher.find()
+        } catch (e: RuntimeException) {
+            false
         }
     }
 
@@ -296,6 +352,42 @@ class VariableExpander {
         return parts
     }
 
+    /**
+     * Unary existence checks (imported from Tasker's "Is Set"/"Not Set" condition ops, which have
+     * no right-hand operand). Suffix-anchored, not scanned like the binary operators, so a
+     * variable value that happens to contain "is_set"/"not_set" mid-string can't false-match.
+     * Returns null when `normalized` doesn't end with either suffix at all, so the caller can
+     * fall through to its own default (`normalized.toBoolean()`).
+     *
+     * This evaluator has two call paths with different pre-expansion behavior:
+     * TaskRunner.evaluateConditionString expands the whole condition once before calling in, but
+     * VariableStore.evaluateCondition (used directly by callers/tests that don't go through
+     * TaskRunner) does not expand at all -- `normalized` can arrive as raw `%variable` text. The
+     * binary comparison branch stays correct under both by calling expandInternal on its operands
+     * regardless of whether the caller already did; this needs the same self-sufficiency, so it
+     * must not assume expansion already happened.
+     *
+     * Empty (post-expansion) operand is a real, meaningful case, not a malformed condition: when
+     * the source variable is unset and the caller pre-expanded (the TaskRunner path), e.g.
+     * "%text not_set" arrives as just " not_set" (empty value + the literal suffix), and
+     * `cond.trim()` at the top of [evaluateConditionInternal] then eats that boundary space -- so
+     * an empty-operand match and a bare "not_set"/"is_set" with nothing before it are
+     * indistinguishable by the time we see them, and both correctly mean "the value is empty".
+     */
+    private fun unaryExistenceResult(normalized: String, store: VariableStore, arrays: ArrayStore): Boolean? {
+        for ((suffix, wantsNonEmpty) in UNARY_EXISTENCE_SUFFIXES) {
+            val bareSuffix = suffix.trim()
+            if (normalized.equals(bareSuffix, ignoreCase = true)) {
+                return !wantsNonEmpty
+            }
+            if (normalized.endsWith(suffix, ignoreCase = true)) {
+                val operand = expandInternal(normalized.removeSuffix(suffix).trim(), store, arrays)
+                return if (wantsNonEmpty) operand.isNotEmpty() else operand.isEmpty()
+            }
+        }
+        return null
+    }
+
     private fun stripOuterParens(expr: String): String {
         var result = expr
         while (result.length >= 2 && result.first() == '(' && matchingCloseParen(result, 0) == result.lastIndex) {
@@ -328,7 +420,7 @@ class VariableExpander {
                 ')' -> if (depth > 0) depth--
             }
             if (depth == 0) {
-                val operator = COMPARISON_OPERATORS.firstOrNull { expr.startsWith(it.token, index) }
+                val operator = COMPARISON_OPERATORS.firstOrNull { expr.matchesOperatorAt(it, index) }
                 if (operator != null) {
                     val left = expr.substring(0, index).trim()
                     val right = expr.substring(index + operator.token.length).trim()
@@ -343,6 +435,27 @@ class VariableExpander {
         return matches.singleOrNull()
     }
 
+    /**
+     * True if `operator`'s token occurs at `index`, and -- for [ComparisonOperator.MATCHES] /
+     * [ComparisonOperator.NOT_MATCHES] only -- is bounded by whitespace or a string edge on both
+     * sides. `~` and `!~` are ordinary characters in real values in a way `==`/`<`/etc. are not
+     * (paths like `~/backups`, version strings like `1.2~rc1`, approximations like `~100`), so
+     * without this a literal tilde inside an otherwise unambiguous comparison's left/right text
+     * makes parseComparison find two operator matches instead of one, `matches.singleOrNull()`
+     * returns null, and the whole comparison silently falls back to `normalized.toBoolean()`
+     * (false) -- turning a working condition into one that's always false. The other operators
+     * don't get this treatment: it would be a larger, non-additive behavior change for tokens
+     * this project already shipped with, out of scope for this fix.
+     */
+    private fun String.matchesOperatorAt(operator: ComparisonOperator, index: Int): Boolean {
+        if (!startsWith(operator.token, index)) return false
+        if (operator != ComparisonOperator.MATCHES && operator != ComparisonOperator.NOT_MATCHES) return true
+        val before = index == 0 || this[index - 1].isWhitespace()
+        val afterIndex = index + operator.token.length
+        val after = afterIndex >= length || this[afterIndex].isWhitespace()
+        return before && after
+    }
+
     private fun compareNumbers(left: String, right: String, predicate: (Double, Double) -> Boolean): Boolean {
         val l = left.toDoubleOrNull() ?: return false
         val r = right.toDoubleOrNull() ?: return false
@@ -427,15 +540,26 @@ class VariableExpander {
         GE(">="),
         LT("<"),
         GT(">"),
+        MATCHES("~"),
+        NOT_MATCHES("!~"),
     }
 
     companion object {
+        // (suffix, isSetWhenTrue) — order matters: "not_set"/"!is_set" style negatives must be
+        // checked before a shorter positive suffix could partially match, though with these two
+        // literal strings neither is a suffix of the other, so this is just future-proofing.
+        private val UNARY_EXISTENCE_SUFFIXES = listOf(
+            " is_set" to true,
+            " not_set" to false,
+        )
         private val COMPARISON_OPERATORS = listOf(
             ComparisonOperator.EQ,
             ComparisonOperator.NE,
             ComparisonOperator.LE,
             ComparisonOperator.GE,
             ComparisonOperator.LT,
+            ComparisonOperator.NOT_MATCHES,
+            ComparisonOperator.MATCHES,
             ComparisonOperator.GT,
         )
         private const val MAX_REGEX_LENGTH = 256

app/src/main/java/com/opentasker/core/transfer/TaskerXmlImport.kt

@@ -426,7 +426,26 @@ object TaskerXmlImporter {
             )
             else -> ActionWithLoss(unsupportedAction(code))
         }
-        val action = actionWithLoss.action
+        // Real Tasker exports carry a "Run only if" guard as a sibling <ConditionList>, not as
+        // <Str> action args -- this applies to any action type, not just flow-control If/Else If.
+        // For most actions "condition" isn't a real arg key, so the parsed value only needs to
+        // land on the generic action.condition field. flow.if is the exception: its own args map
+        // already carries the old flat-<Str> fallback under "condition" (see the "37"/"if" branch
+        // above), and that same key is what both TaskRunner's stepControl and the action editor's
+        // existingActionArgValue() read as the if's actual test expression -- so when this action
+        // type already has an args["condition"] entry, overwrite it with the real parsed condition
+        // instead of dropping it, or the editor shows a blank required field for a working import.
+        val (importedCondition, conditionWarning) = element.parseImportedCondition()
+        val action = if (importedCondition != null) {
+            val args = if (actionWithLoss.action.args.containsKey("condition")) {
+                actionWithLoss.action.args + ("condition" to importedCondition)
+            } else {
+                actionWithLoss.action.args
+            }
+            actionWithLoss.action.copy(condition = importedCondition, args = args)
+        } else {
+            actionWithLoss.action
+        }
         val unsupported = if (action.type == TASKER_UNSUPPORTED_ACTION_ID) {
             TaskerUnsupportedAction(taskName = taskName, taskerCode = code, actionIndex = actionIndex)
         } else {
@@ -441,7 +460,9 @@ object TaskerXmlImporter {
             action = action,
             mapped = mapped,
             unsupported = unsupported,
-            lossyWarning = actionWithLoss.lossyWarning,
+            lossyWarning = listOfNotNull(actionWithLoss.lossyWarning, conditionWarning)
+                .joinToString("; ")
+                .ifBlank { null },
         )
     }
 
@@ -560,6 +581,55 @@ object TaskerXmlImporter {
     private fun Element.argIndex(): Int =
         getAttribute("sr").filter(Char::isDigit).toIntOrNull() ?: Int.MAX_VALUE
 
+    private data class ImportedCondition(val expression: String?, val warning: String?)
+
+    /**
+     * Reads a Tasker `<ConditionList sr="if"><Condition><lhs>/<op>/<rhs></Condition></ConditionList>`
+     * -- real Tasker exports encode the "Run only if" guard this way on ANY action, not just
+     * flow-control If/Else If (measured on a real backup: 85 of 118 ConditionList occurrences were
+     * on ordinary actions like Set Variable). Returns a condition string in this app's own syntax,
+     * or a null expression if the action has no ConditionList, which is the normal case for most
+     * actions.
+     *
+     * Only the single-condition case is handled: every real sample examined had exactly one
+     * `<Condition>` per list. Tasker does support AND/OR chains of multiple conditions via extra
+     * `<Condition>`/`<Bool>` siblings; a multi-condition list degrades to using only the first
+     * condition (rather than silently producing "true" the way every ConditionList case did before
+     * this fix) and reports that degradation via [ImportedCondition.warning]. Likewise, an `<op>`
+     * code outside the known Tasker set (0-9, 12, 13) yields a null expression -- which for flow.if
+     * specifically falls back to the old literal-"true" behavior -- but is now reported instead of
+     * silently reproduced.
+     */
+    private fun Element.parseImportedCondition(): ImportedCondition {
+        val conditionList = directChildren("ConditionList").firstOrNull() ?: return ImportedCondition(null, null)
+        val conditions = conditionList.directChildren("Condition")
+        val condition = conditions.firstOrNull() ?: return ImportedCondition(null, null)
+        val lhs = condition.childText("lhs")
+        if (lhs.isBlank()) return ImportedCondition(null, null)
+        val multiConditionWarning = if (conditions.size > 1) {
+            "a multi-condition \"Run only if\" guard was reduced to just its first condition"
+        } else {
+            null
+        }
+        val op = condition.childText("op")
+        val expression = when (op) {
+            "12" -> "$lhs is_set"
+            "13" -> "$lhs not_set"
+            else -> {
+                val token = TASKER_CONDITION_OP_TOKENS[op]
+                    ?: return ImportedCondition(
+                        expression = null,
+                        warning = listOfNotNull(
+                            multiConditionWarning,
+                            "a \"Run only if\" guard used an unsupported Tasker comparison (op $op) and was dropped",
+                        ).joinToString("; "),
+                    )
+                "$lhs $token ${condition.childText("rhs")}"
+            }
+        }
+        return ImportedCondition(expression, multiConditionWarning)
+    }
+
     private fun org.w3c.dom.NodeList.asElementList(): List<Element> =
             (0 until length).mapNotNull { index -> item(index).takeIf { it.nodeType == Node.ELEMENT_NODE } as? Element }
 
@@ -600,4 +670,26 @@ object TaskerXmlImporter {
     )
 
     const val TASKER_UNSUPPORTED_ACTION_ID = "tasker.unsupported"
+
+    // Tasker's numeric Condition <op> codes, sourced from github.com/mctinker/Map-Tasker's
+    // IF_CONDITION_OPERATORS table (a mature, independently-verified Tasker XML tool) and
+    // cross-checked against a real backup: op values 0/1/2/12/13 all appear and match their
+    // documented semantics -- e.g. the one real op=1 instance compares %http_response_code
+    // against 200, matching this project's own documented "Doesn't Match 200" HTTP-status check.
+    // 3-9 don't appear in that corpus but are included for completeness. 8/9 are the "(Numeric)"
+    // variants of =/!=; this evaluator has no numeric-aware equality distinct from string
+    // equality, so they map to the same tokens as 0/1. 12/13 (Is Set/Not Set) are unary and
+    // handled separately in parseImportedCondition, not through this map.
+    private val TASKER_CONDITION_OP_TOKENS = mapOf(
+        "0" to "==",
+        "1" to "!=",
+        "2" to "~",
+        "3" to "!~",
+        "4" to "~",
+        "5" to "!~",
+        "6" to "<",
+        "7" to ">",
+        "8" to "==",
+        "9" to "!=",
+    )
 }