Merge pull request #4 from appdevforall/chore/phase0-guardrails

chore(controller): Phase 0 tech-debt guardrails — tests + CI gate

Author: luisguzman-adfa

.github/workflows/android-sanity-check.yml

@@ -31,8 +31,16 @@ jobs:
     - name: Grant Execute Permission to Gradlew
       run: chmod +x gradlew
 
-#    - name: Run Android Lint (Syntax & Rules Check)
-#      run: ./gradlew lintDebug
+    # Phase 0 safety net: unit tests are a BLOCKING gate.
+    - name: Run Unit Tests
+      run: ./gradlew testDebugUnitTest --stacktrace
+
+    # Lint runs for visibility only and never blocks the job (continue-on-error).
+    # Scoped to :app so the vendored termux-core/upstream lint backlog does not fail CI.
+    # Once the :app lint backlog is triaged, set abortOnError=true and drop continue-on-error.
+    - name: Run Android Lint (reporting, non-blocking)
+      continue-on-error: true
+      run: ./gradlew :app:lintDebug
 
     - name: Compile Test (Assemble Debug)
       run: ./gradlew assembleDebug --stacktrace

.gitignore

@@ -0,0 +1,9 @@
+# IDE / editor
+.idea/
+*.iml
+*.iws
+*.ipr
+
+# OS
+.DS_Store
+Thumbs.db

controller/app/build.gradle

@@ -105,6 +105,14 @@ android {
         abortOnError false
     }
 
+    testOptions {
+        unitTests {
+            // Let JVM unit tests stub Android framework calls (e.g. android.util.Log)
+            // so framework-free logic can be tested without an emulator.
+            returnDefaultValues = true
+        }
+    }
+
     dependenciesInfo {
         includeInApk = false
         includeInBundle = false
@@ -149,6 +157,8 @@ dependencies {
 
     // Testing
     testImplementation 'junit:junit:4.13.2'
+    // Real org.json so SyncHandshakeHelper JSON logic can run in JVM unit tests
+    testImplementation 'org.json:json:20231013'
     androidTestImplementation 'androidx.test.espresso:espresso-core:3.5.1'
     androidTestImplementation 'androidx.test.ext:junit:1.1.5'
 }

controller/app/src/main/java/org/iiab/controller/DashboardFragment.java

@@ -567,12 +567,7 @@ private boolean pingUrl(String urlStr) {
 
     // Extracts the numbers (in kB) from the lines of /proc/meminfo
     private long parseMemLine(String line) {
-        try {
-            String[] parts = line.split("\\s+");
-            return Long.parseLong(parts[1]);
-        } catch (Exception e) {
-            return 0;
-        }
+        return SystemStatsUtil.parseMemLine(line);
     }
 
     // --- METHODS FOR OBTAINING IPs ---
@@ -719,13 +714,7 @@ private String getTermuxArch() {
     }
 
     private String getDebianArch(String androidArch) {
-        if (androidArch == null || androidArch.equals("N/A")) return "N/A";
-        String lower = androidArch.toLowerCase();
-
-        if (lower.contains("arm64") || lower.contains("aarch64")) return "arm64";
-        if (lower.contains("armeabi") || lower.contains("armv7")) return "armhf";
-
-        return lower;
+        return SystemStatsUtil.getDebianArch(androidArch);
     }
 
     // Converter from DP to actual screen pixels

controller/app/src/main/java/org/iiab/controller/SystemStatsUtil.java

@@ -0,0 +1,60 @@
+package org.iiab.controller;
+
+/**
+ * Framework-free helpers for parsing system statistics.
+ *
+ * <p>These functions were extracted from {@code DashboardFragment} so they can be
+ * unit-tested on the plain JVM (no Android dependencies, no emulator). Keep this
+ * class free of any {@code android.*} imports.
+ */
+public final class SystemStatsUtil {
+
+    private SystemStatsUtil() {
+        // Utility class — no instances.
+    }
+
+    /**
+     * Parses a single {@code /proc/meminfo} line and returns the numeric value (in kB).
+     *
+     * <p>A meminfo line looks like {@code "MemTotal:        8127200 kB"}. The value is the
+     * second whitespace-separated token. Returns {@code 0} for any malformed or null input
+     * rather than throwing, preserving the original defensive behavior.
+     *
+     * @param line a line from {@code /proc/meminfo}, e.g. {@code "MemAvailable:   123456 kB"}
+     * @return the parsed value in kB, or {@code 0} if the line cannot be parsed
+     */
+    public static long parseMemLine(String line) {
+        if (line == null) {
+            return 0;
+        }
+        try {
+            String[] parts = line.trim().split("\\s+");
+            return Long.parseLong(parts[1]);
+        } catch (Exception e) {
+            return 0;
+        }
+    }
+
+    /**
+     * Maps an Android/Termux architecture string to the matching Debian architecture name.
+     *
+     * @param androidArch the Android architecture (e.g. {@code "aarch64"}, {@code "armv7l"})
+     * @return the Debian architecture ({@code "arm64"}, {@code "armhf"}), {@code "N/A"} when
+     *         unknown/empty, or the lower-cased input when no mapping applies
+     */
+    public static String getDebianArch(String androidArch) {
+        if (androidArch == null || androidArch.equals("N/A")) {
+            return "N/A";
+        }
+        String lower = androidArch.toLowerCase();
+
+        if (lower.contains("arm64") || lower.contains("aarch64")) {
+            return "arm64";
+        }
+        if (lower.contains("armeabi") || lower.contains("armv7")) {
+            return "armhf";
+        }
+
+        return lower;
+    }
+}

controller/app/src/test/java/org/iiab/controller/SyncHandshakeHelperTest.java

@@ -0,0 +1,82 @@
+package org.iiab.controller;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Test;
+
+/**
+ * Unit tests for the pure (framework-free) parts of {@link SyncHandshakeHelper}:
+ * password generation and the QR payload create/parse round-trip.
+ *
+ * <p>Relies on {@code testOptions.unitTests.returnDefaultValues = true} (so
+ * {@code android.util.Log} calls no-op) and the real {@code org.json} test dependency.
+ */
+public class SyncHandshakeHelperTest {
+
+    // --- generateSecurePassword ---
+
+    @Test
+    public void generateSecurePassword_hasExpectedLength() {
+        assertEquals(12, SyncHandshakeHelper.generateSecurePassword().length());
+    }
+
+    @Test
+    public void generateSecurePassword_usesOnlyAlphanumericChars() {
+        String pwd = SyncHandshakeHelper.generateSecurePassword();
+        assertTrue("password should be alphanumeric: " + pwd, pwd.matches("[A-Za-z0-9]+"));
+    }
+
+    @Test
+    public void generateSecurePassword_isNotConstant() {
+        // Extremely unlikely to collide; guards against a degenerate generator.
+        assertTrue(!SyncHandshakeHelper.generateSecurePassword()
+                .equals(SyncHandshakeHelper.generateSecurePassword()));
+    }
+
+    // --- createPayload / parsePayload round-trip ---
+
+    @Test
+    public void payload_roundTripsAllFields() {
+        String payload = SyncHandshakeHelper.createPayload(
+                "192.168.1.50", 8730, "iiab_peer", "s3cretPass", true, 64);
+
+        SyncHandshakeHelper.SyncCredentials creds = SyncHandshakeHelper.parsePayload(payload);
+
+        assertNotNull(creds);
+        assertEquals("192.168.1.50", creds.ip);
+        assertEquals(8730, creds.port);
+        assertEquals("iiab_peer", creds.user);
+        assertEquals("s3cretPass", creds.pass);
+        assertTrue(creds.hasRootfs);
+        assertEquals(64, creds.archBits);
+    }
+
+    @Test
+    public void parsePayload_returnsNullForNonIiabJson() {
+        assertNull(SyncHandshakeHelper.parsePayload("{\"app\":\"some_other_app\"}"));
+    }
+
+    @Test
+    public void parsePayload_returnsNullForMalformedJson() {
+        assertNull(SyncHandshakeHelper.parsePayload("this is not json"));
+    }
+
+    @Test
+    public void parsePayload_returnsNullForNull() {
+        assertNull(SyncHandshakeHelper.parsePayload(null));
+    }
+
+    @Test
+    public void parsePayload_defaultsHasRootfsTrueForLegacyPayload() {
+        // Legacy payloads without "has_rootfs" should default to true.
+        String legacy = "{\"app\":\"iiab_sync\",\"ip\":\"10.0.0.1\",\"port\":8730,"
+                + "\"user\":\"u\",\"pass\":\"p\"}";
+        SyncHandshakeHelper.SyncCredentials creds = SyncHandshakeHelper.parsePayload(legacy);
+        assertNotNull(creds);
+        assertTrue(creds.hasRootfs);
+        assertEquals(0, creds.archBits);
+    }
+}

controller/app/src/test/java/org/iiab/controller/SystemStatsUtilTest.java

@@ -0,0 +1,86 @@
+package org.iiab.controller;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+/**
+ * Unit tests for {@link SystemStatsUtil}. These run on the plain JVM — no emulator.
+ * This is the first safety-net test added in Phase 0 of the tech-debt remediation plan.
+ */
+public class SystemStatsUtilTest {
+
+    // --- parseMemLine ---
+
+    @Test
+    public void parseMemLine_parsesStandardMeminfoLine() {
+        assertEquals(8127200L, SystemStatsUtil.parseMemLine("MemTotal:        8127200 kB"));
+    }
+
+    @Test
+    public void parseMemLine_parsesAvailableLine() {
+        assertEquals(123456L, SystemStatsUtil.parseMemLine("MemAvailable:   123456 kB"));
+    }
+
+    @Test
+    public void parseMemLine_handlesLeadingAndTrailingWhitespace() {
+        assertEquals(42L, SystemStatsUtil.parseMemLine("   SwapFree:   42 kB   "));
+    }
+
+    @Test
+    public void parseMemLine_returnsZeroForNull() {
+        assertEquals(0L, SystemStatsUtil.parseMemLine(null));
+    }
+
+    @Test
+    public void parseMemLine_returnsZeroForEmpty() {
+        assertEquals(0L, SystemStatsUtil.parseMemLine(""));
+    }
+
+    @Test
+    public void parseMemLine_returnsZeroWhenSecondTokenNotNumeric() {
+        assertEquals(0L, SystemStatsUtil.parseMemLine("MemTotal: notANumber kB"));
+    }
+
+    @Test
+    public void parseMemLine_returnsZeroWhenNoSecondToken() {
+        assertEquals(0L, SystemStatsUtil.parseMemLine("MemTotal:"));
+    }
+
+    // --- getDebianArch ---
+
+    @Test
+    public void getDebianArch_mapsAarch64ToArm64() {
+        assertEquals("arm64", SystemStatsUtil.getDebianArch("aarch64"));
+    }
+
+    @Test
+    public void getDebianArch_mapsArm64VariantToArm64() {
+        assertEquals("arm64", SystemStatsUtil.getDebianArch("ARM64-v8a"));
+    }
+
+    @Test
+    public void getDebianArch_mapsArmeabiToArmhf() {
+        assertEquals("armhf", SystemStatsUtil.getDebianArch("armeabi-v7a"));
+    }
+
+    @Test
+    public void getDebianArch_mapsArmv7ToArmhf() {
+        assertEquals("armhf", SystemStatsUtil.getDebianArch("armv7l"));
+    }
+
+    @Test
+    public void getDebianArch_returnsNaForNull() {
+        assertEquals("N/A", SystemStatsUtil.getDebianArch(null));
+    }
+
+    @Test
+    public void getDebianArch_returnsNaForNaSentinel() {
+        assertEquals("N/A", SystemStatsUtil.getDebianArch("N/A"));
+    }
+
+    @Test
+    public void getDebianArch_lowercasesUnknownArch() {
+        assertEquals("x86_64", SystemStatsUtil.getDebianArch("X86_64"));
+    }
+}

controller/docs/FORK_DELTA_ANALYSIS.md

@@ -0,0 +1,44 @@
+# Termux Fork — Delta Analysis (iiab changes vs upstream)
+
+> Scope: technical debt **only in what iiab modified** in the vendored Termux fork at `controller/termux-core/termux-source` (submodule → `github.com/iiab/termux-app`). Upstream code is out of scope. Date: 2026-06-16.
+
+## 1. What iiab actually changed
+
+The fork's HEAD (`f8f36614`) sits **8 commits** above upstream base `30ebb2de`, all authored by `Ark74`. The **net delta is a single method** — `ExtraKeysView.loadIIABDefaultKeys()`, +25 lines in:
+
+```
+termux-shared/src/main/java/com/termux/shared/termux/extrakeys/ExtraKeysView.java
+```
+
+It builds a hardcoded extra-keys layout and calls the existing public `reload(ExtraKeysInfo, float)`. It is called once, from `controller/app/.../MainActivity.java:2036`, right before wiring the extra-keys click listener.
+
+The 8 commits are: a 44-line feature commit, a "decouple" commit that removed 24 lines, and six small "fix: yet another change pt2…pt5" tweaks that net to the final 25-line method.
+
+## 2. Findings (scoped to the delta)
+
+Scoring: **Priority = (Impact + Risk) × (6 − Effort)**, each 1–5.
+
+| ID | Location | Category | Issue | Imp | Risk | Eff | Prio |
+|----|----------|----------|-------|----|----|----|----|
+| K1 | `ExtraKeysView.java:682–706` | Architecture / fork maintenance | The change lives **inside an upstream file** but uses only public APIs (`reload`, public `ExtraKeysInfo` ctor, public `ExtraKeyDisplayMap`). The same result is achievable entirely from the controller app — so the fork need not modify upstream source at all. Every upstream sync will now conflict on this file. | 4 | 3 | 2 | 28 |
+| K2 | 8 commits `e6c7b88d..f8f36614` | Documentation / process | Unreviewable history: five commits named `fix: yet another change pt2…pt5` with no body, netting 25 lines. Can't bisect, cherry-pick, or review intent. | 3 | 2 | 1 | 25 |
+| K3 | `ExtraKeysView.java:688–692` | Code | Keyboard layout is a **hardcoded inline pseudo-JSON string** in Java; the comment "we match the same Termux keys" admits manual drift. Duplicates Termux's normal properties-driven layout and the controller's own ESC/TAB/… key-handling switch (`MainActivity.java:~2040+`). | 2 | 2 | 2 | 16 |
+| K4 | `ExtraKeysView.java:702–704` | Code | Broad `catch (Exception e)` logs and continues; if layout construction fails the user silently gets **no extra keys** with no fallback to upstream defaults. | 2 | 2 | 1 | 20 |
+| K5 | `ExtraKeysView.java:686` (no test) | Test | The layout string's validity (parses into a valid `ExtraKeysInfo`) is untested. Once the literal moves into the app as a constant, it becomes a trivial JVM unit test. | 2 | 2 | 2 | 16 |
+| K6 | `ExtraKeysView.java:682–686, 703` | Code (cosmetic) | Malformed Javadoc (`/**` at column 0 while body is indented), decorative `===` banner comments, and fully-qualified `android.util.Log` instead of an import. The inline-FQN/no-import-change is actually a reasonable merge-conflict-minimization tactic — only matters once the code relocates. | 1 | 1 | 1 | 10 |
+
+## 3. Top recommendation — relocate the change out of upstream (K1)
+
+This single move resolves the core maintainability problem and directly supports the submodule/build work just completed.
+
+1. Move the body of `loadIIABDefaultKeys()` into the controller app — e.g. a small helper that builds the `ExtraKeysInfo` (layout as a named constant or `R.string`/resource, addressing **K3**) and calls `extraKeysView.reload(iiabKeysInfo, 0f)` directly. `MainActivity.java:2036` already holds the `extraKeysView` reference, so the call site barely changes.
+2. Revert `ExtraKeysView.java` to its upstream contents and **pin the submodule to a clean upstream tag**. The fork becomes a pristine mirror → upstream upgrades stop conflicting.
+3. Add a fallback (K4): if the custom layout fails to build, fall back to Termux's default keys rather than showing none.
+4. Squash the 8 commits into one well-described commit before any further sharing (**K2**).
+5. Add the layout-validity unit test once the constant lives in the app (**K5**), fitting the Phase 0 safety-net pattern.
+
+Net effect: **zero iiab modifications to vendored upstream**, a single-source-of-truth layout shared with the controller's key handling, and a fork that tracks upstream cleanly.
+
+## 4. Note
+
+The earlier `controller/app` analysis (`TECH_DEBT_PLAN.md`) is unaffected by the submodule now being present — those 34 files are unchanged. The submodule only matters for building (dependency resolution), which is handled on-device via Android Studio and in CI.