ci: route Maestro e2e selection through sniffler impact analysis

.github/README.md

@@ -6,7 +6,7 @@ Maps which event triggers which workflow and how they call each other.
 
 | Workflow | Trigger | What runs |
 |---|---|---|
-| [build-pr.yml](workflows/build-pr.yml) | `pull_request` (all branches) | Lint + tests, PR changelog, Android + iOS store builds (gated), E2E build + Maestro shards on both platforms (gated) |
+| [build-pr.yml](workflows/build-pr.yml) | `pull_request` (all branches) | Lint + tests, PR changelog, Android + iOS store builds (gated), E2E build + Maestro shards on both platforms (gated; `e2e-shards` narrows to the sniffler-impacted shards, or skips the stage on a confident-zero diff) |
 | [build-develop.yml](workflows/build-develop.yml) | `push: develop` | Lint + tests, release changelog, Android + iOS store builds, seeds Android AVD + SDK caches for E2E shards |
 | [prettier.yml](workflows/prettier.yml) | `push: * except master, develop, single-server` (main repo) | Auto-formats with Oxfmt + Oxlint and commits any fixes back to the branch |
 | [organize_translations.yml](workflows/organize_translations.yml) | `push` touching `app/i18n/locales/**.json` | Sorts JSON keys and commits the result |
@@ -18,6 +18,7 @@ flowchart TD
     classDef entry fill:#d4e6f1,stroke:#2980b9
     classDef reusable fill:#d5f5e3,stroke:#27ae60
     classDef action fill:#fef9e7,stroke:#f39c12
+    classDef gate fill:#f5e6f8,stroke:#8e44ad
 
     PR([build-pr.yml]):::entry
     DEV([build-develop.yml]):::entry
@@ -44,13 +45,22 @@ flowchart TD
     PREAND[preinstall-android-sdk]:::action
     E2EACC[e2e-account]:::action
 
+    ESHARD[e2e-shards preflight]:::gate
+    ERESULT[e2e-result required check]:::gate
+
     PR --> ESLINT
     PR --> BUILDAND
     PR --> BUILDIOS
-    PR --> E2EAND
-    PR --> E2EIOS
-    PR --> MASTAND
-    PR --> MASIOS
+    PR --> ESHARD
+
+    ESHARD -->|should_run| E2EAND
+    ESHARD -->|should_run| E2EIOS
+    ESHARD -->|should_run| MASTAND
+    ESHARD -->|should_run| MASIOS
+
+    MASTAND --> ERESULT
+    MASIOS --> ERESULT
+    ESHARD --> ERESULT
 
     DEV --> ESLINT
     DEV --> CHANGELOG
@@ -90,4 +100,4 @@ flowchart TD
 | `android_build` | [build-android.yml](workflows/build-android.yml) — `build-hold` | Called with `trigger == pr` (i.e. from `build-pr.yml`) |
 | `upload_android` | [build-android.yml](workflows/build-android.yml) — `upload-hold` | Called with `trigger == pr`, after the Android build completes |
 | `ios_build` | [build-ios.yml](workflows/build-ios.yml) — `build-hold` | Called with `trigger == pr` (i.e. from `build-pr.yml`) |
-| `approve_e2e_testing` | [build-pr.yml](workflows/build-pr.yml) — `e2e-hold` | Every `pull_request` run |
+| `approve_e2e_testing` | [build-pr.yml](workflows/build-pr.yml) — `e2e-hold` | A `pull_request` run whose diff impacts at least one Maestro flow (`e2e-shards` sets `should_run == true`). A confident-zero diff skips the whole e2e stage, so no approval fires. |

.github/scripts/__tests__/changed-runner.test.js

@@ -0,0 +1,198 @@
+// Tests for e2e-changed.sh: the local `pnpm e2e:changed <android|ios>` runner.
+// Unlike select-impacted-shards.sh, this script never writes to $GITHUB_OUTPUT —
+// it is a pure CLI that gathers a CHANGED file set from git and execs straight
+// into `pnpm exec sniffler run --changed ... -- maestro test ...`. Maestro is
+// stubbed throughout: these tests prove the arg validation, the maestro guard,
+// the merge-base fallback, and the CHANGED-gathering/invocation shape — actual
+// Maestro flow execution needs a booted device and is out of scope here.
+'use strict';
+
+const path = require('path');
+const { runScript } = require('../testlib/runScript');
+
+const SCRIPT = path.join(__dirname, '..', 'e2e-changed.sh');
+
+// No-op maestro: its presence alone satisfies the `command -v maestro` guard.
+const MAESTRO_NOOP = 'exit 0';
+
+// Records its own invocation to stdout so the trailing `maestro test ...`
+// tail (once sniffler forwards to it) shows up in the captured process output.
+const MAESTRO_RECORDING = `echo "MAESTRO_ARGS:$*"\nexit 0`;
+
+// git stub: branches on the subcommand the script actually calls
+// (merge-base / diff / ls-files). `mergeBase: null` simulates the
+// unresolved-merge-base failure the script falls back on.
+function gitStub({ mergeBase = 'deadbeef', diffFiles = [], untrackedFiles = [] } = {}) {
+	const diffBody = diffFiles.map(f => `echo '${f}'`).join('\n\t\t') || ':';
+	const untrackedBody = untrackedFiles.map(f => `echo '${f}'`).join('\n\t\t') || ':';
+	return `
+case "$1" in
+	merge-base)
+		${mergeBase === null ? 'exit 1' : `echo '${mergeBase}'`}
+		;;
+	diff)
+		${diffBody}
+		;;
+	ls-files)
+		${untrackedBody}
+		;;
+	*)
+		exit 0
+		;;
+esac
+`;
+}
+
+// pnpm stub: only intercepts `pnpm exec sniffler ...` (the script's real
+// invocation shape). Echoes its own args (proves the --changed set / tail
+// command sniffler received), then either reports a confident zero or execs
+// the trailing `maestro test ...` command so it shows up in stdout too.
+function pnpmStub({ zero = false } = {}) {
+	return `
+if [ "$1 $2" = "exec sniffler" ]; then
+	echo "PNPM_ARGS:$*"
+	if [ "${zero}" = "true" ]; then
+		echo "sniffler: no impacted flows (confident zero)"
+		exit 0
+	fi
+	while [ $# -gt 0 ] && [ "$1" != "--" ]; do
+		shift
+	done
+	shift
+	exec "$@"
+fi
+exit 0
+`;
+}
+
+describe('e2e-changed.sh', () => {
+	describe('platform arg validation', () => {
+		test('missing platform arg prints usage and exits 2', () => {
+			const result = runScript(SCRIPT, { args: [] });
+			expect(result.status).toBe(2);
+			expect(result.stderr).toContain('usage: pnpm e2e:changed <android|ios>');
+		});
+
+		test('invalid platform arg prints usage and exits 2', () => {
+			const result = runScript(SCRIPT, { args: ['windows'] });
+			expect(result.status).toBe(2);
+			expect(result.stderr).toContain('usage: pnpm e2e:changed <android|ios>');
+		});
+	});
+
+	describe('missing-maestro guard', () => {
+		test('valid platform but no maestro on PATH fires the guard', () => {
+			// Override PATH to a maestro-free set of dirs (no binDir stub either —
+			// the guard fires before any git/sniffler call, so none is needed).
+			const result = runScript(SCRIPT, {
+				args: ['android'],
+				env: { PATH: '/usr/bin:/bin:/opt/homebrew/bin' }
+			});
+			expect(result.status).toBe(2);
+			expect(result.stderr).toContain('ERROR: maestro not found in PATH');
+		});
+	});
+
+	describe('merge-base failure fallback', () => {
+		test('unresolved merge-base against the default base exits 1 with an actionable error', () => {
+			const result = runScript(SCRIPT, {
+				args: ['android'],
+				stubs: { maestro: MAESTRO_NOOP, git: gitStub({ mergeBase: null }) }
+			});
+			expect(result.status).toBe(1);
+			expect(result.stderr).toContain("cannot resolve merge-base against 'origin/develop'");
+			expect(result.stderr).toContain('set E2E_BASE');
+		});
+
+		test('E2E_BASE override is reflected in the failure message', () => {
+			const result = runScript(SCRIPT, {
+				args: ['ios'],
+				env: { E2E_BASE: 'origin/custom-base' },
+				stubs: { maestro: MAESTRO_NOOP, git: gitStub({ mergeBase: null }) }
+			});
+			expect(result.status).toBe(1);
+			expect(result.stderr).toContain("cannot resolve merge-base against 'origin/custom-base'");
+		});
+	});
+
+	describe('CHANGED set gathering', () => {
+		test('android: committed + uncommitted-tracked + untracked files are deduped, sorted, and forwarded to sniffler', () => {
+			const result = runScript(SCRIPT, {
+				args: ['android'],
+				stubs: {
+					maestro: MAESTRO_RECORDING,
+					git: gitStub({
+						diffFiles: ['app/views/RoomView.tsx', 'app/actions/room.ts', 'app/actions/room.ts'],
+						untrackedFiles: ['app/actions/room.ts', 'app/views/NewFeature.tsx']
+					}),
+					pnpm: pnpmStub()
+				}
+			});
+			expect(result.status).toBe(0);
+			expect(result.stdout).toContain(
+				'PNPM_ARGS:exec sniffler run --changed ' +
+					'app/actions/room.ts app/views/NewFeature.tsx app/views/RoomView.tsx -- ' +
+					'maestro test -e APP_ID=chat.rocket.android --exclude-tags=util --exclude-tags=ios-only'
+			);
+		});
+
+		test('ios: platform selects the ios APP_ID and excludes android-only flows', () => {
+			const result = runScript(SCRIPT, {
+				args: ['ios'],
+				stubs: {
+					maestro: MAESTRO_RECORDING,
+					git: gitStub({ diffFiles: ['app/views/RoomView.tsx'] }),
+					pnpm: pnpmStub()
+				}
+			});
+			expect(result.status).toBe(0);
+			expect(result.stdout).toContain(
+				'PNPM_ARGS:exec sniffler run --changed app/views/RoomView.tsx -- ' +
+					'maestro test -e APP_ID=chat.rocket.ios --exclude-tags=util --exclude-tags=android-only'
+			);
+		});
+
+		test('sniffler forwards to maestro, which is invoked with the built command tail', () => {
+			const result = runScript(SCRIPT, {
+				args: ['android'],
+				stubs: {
+					maestro: MAESTRO_RECORDING,
+					git: gitStub({ diffFiles: ['app/views/RoomView.tsx'] }),
+					pnpm: pnpmStub()
+				}
+			});
+			expect(result.status).toBe(0);
+			expect(result.stdout).toContain(
+				'MAESTRO_ARGS:test -e APP_ID=chat.rocket.android --exclude-tags=util --exclude-tags=ios-only'
+			);
+		});
+	});
+
+	describe('confident zero', () => {
+		test('no changes at all vs base exits 0 cleanly without calling sniffler', () => {
+			const result = runScript(SCRIPT, {
+				args: ['android'],
+				stubs: { maestro: MAESTRO_NOOP, git: gitStub() }
+				// no pnpm stub: if the script reached the exec line, the real (unstubbed)
+				// pnpm would run and this test would fail or hang instead of passing.
+			});
+			expect(result.status).toBe(0);
+			expect(result.stdout).toContain('No changes vs origin/develop — nothing to run.');
+			expect(result.stdout).not.toContain('PNPM_ARGS');
+		});
+
+		test('changes exist but sniffler reports no impacted flow: clean exit 0, maestro never runs', () => {
+			const result = runScript(SCRIPT, {
+				args: ['android'],
+				stubs: {
+					maestro: MAESTRO_RECORDING,
+					git: gitStub({ diffFiles: ['app/views/RoomView.tsx'] }),
+					pnpm: pnpmStub({ zero: true })
+				}
+			});
+			expect(result.status).toBe(0);
+			expect(result.stdout).toContain('sniffler: no impacted flows (confident zero)');
+			expect(result.stdout).not.toContain('MAESTRO_ARGS');
+		});
+	});
+});

.github/scripts/__tests__/coverage.test.js

@@ -0,0 +1,92 @@
+// Proves invariant (1) "no under-selection" against the REAL .sniffler/test-map.json
+// and .sniffler/config.json: for each map-assertable diff in scenario-catalog.json
+// (rows C1..C8), replicates sniffler's documented selection semantics in JS —
+// root/ignore filtering, dependsOn glob matching, then flow -> test-N extraction
+// the same way select-impacted-shards.sh does — and asserts the exact shard set.
+// sniffler's own recommendation algorithm is trusted/out-of-scope; this only
+// tests OUR map's globs and OUR config against real flow files on disk.
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+const micromatch = require('micromatch');
+
+const REPO_ROOT = path.resolve(__dirname, '../../..');
+
+const config = require('../../../.sniffler/config.json');
+const testMap = require('../../../.sniffler/test-map.json');
+const catalog = require('./fixtures/scenario-catalog.json');
+
+// Mirrors the grep pattern in select-impacted-shards.sh: `^\s*-\s*['"]?test-N`.
+const TEST_N_PATTERN = /^[ \t]*-[ \t]*['"]?test-(\d+)/gm;
+
+function extractShardsFromFlow(flowPath) {
+	const contents = fs.readFileSync(path.join(REPO_ROOT, flowPath), 'utf8');
+	const shards = new Set();
+	let match;
+	while ((match = TEST_N_PATTERN.exec(contents)) !== null) {
+		shards.add(Number(match[1]));
+	}
+	return shards;
+}
+
+function isUnderSourceRoots(diffPath) {
+	return config.source.roots.some(root => micromatch.isMatch(diffPath, `${root}/**`));
+}
+
+function isIgnored(diffPath) {
+	return config.source.ignore.some(glob => micromatch.isMatch(diffPath, glob));
+}
+
+function matchedFlowsFor(diffPath) {
+	return testMap.filter(entry => entry.dependsOn.some(glob => micromatch.isMatch(diffPath, glob))).map(entry => entry.test);
+}
+
+// Replicates select-impacted-shards.sh's documented happy path against the real
+// map: runAllWhenChanged -> full; else filter by source roots/ignore, match
+// dependsOn globs, then union the matched flows' `- test-N` tags.
+function computeSelection(diffPaths) {
+	const fullShards = [...catalog.fullShards].sort((a, b) => a - b);
+
+	if (diffPaths.some(p => config.tests.runAllWhenChanged.includes(p))) {
+		return { shards: fullShards, shouldRun: true };
+	}
+
+	const survivors = diffPaths.filter(p => isUnderSourceRoots(p) && !isIgnored(p));
+	if (survivors.length === 0) {
+		return { shards: [], shouldRun: false };
+	}
+
+	const matchedFlows = new Set(survivors.flatMap(matchedFlowsFor));
+	if (matchedFlows.size === 0) {
+		return { shards: [], shouldRun: false };
+	}
+
+	const shardSet = new Set();
+	for (const flow of matchedFlows) {
+		for (const shard of extractShardsFromFlow(flow)) shardSet.add(shard);
+	}
+
+	// Defensive: an impacted flow with no derivable tag must fall to full rather
+	// than under-select (mirrors select-impacted-shards.sh's own fallback).
+	if (shardSet.size === 0) {
+		return { shards: fullShards, shouldRun: true };
+	}
+
+	const shards = [...shardSet].sort((a, b) => a - b);
+	return { shards, shouldRun: true };
+}
+
+describe('sniffler shard selection against the real .sniffler map', () => {
+	const scenarios = catalog.scenarios.filter(s => s.assertableIn.includes('map'));
+
+	test('catalog has map-assertable scenarios to run', () => {
+		expect(scenarios.length).toBeGreaterThan(0);
+	});
+
+	test.each(scenarios)('$id: $name', scenario => {
+		const { shards, shouldRun } = computeSelection(scenario.input.diff);
+		expect(shards).toEqual(scenario.expectedShards);
+		expect(shouldRun).toBe(scenario.expectedShouldRun);
+	});
+});

.github/scripts/__tests__/fixtures/flows/no-tag.yaml

@@ -0,0 +1,4 @@
+appId: ${APP_ID}
+name: No Tag Test
+tags:
+  - android-only