diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index 8955ac7..a64e046 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -3,7 +3,7 @@ on: pull_request: paths: ['android/**', 'ui/**', 'scripts/**', 'frame/**', 'apk-catalog/**', 'ios/scripts/make_frame_bundle.py', '.github/workflows/android.yml'] push: - branches: [main, 'feat/phone-screenshot-autosync-108'] + branches: [main] paths: ['android/**', 'ui/**', 'scripts/**', 'frame/**', 'apk-catalog/**', 'ios/scripts/make_frame_bundle.py', '.github/workflows/android.yml'] permissions: contents: read diff --git a/android/app/src/androidTest/java/com/saphid/framecontrol/ScreenshotDeviceTest.java b/android/app/src/androidTest/java/com/saphid/framecontrol/ScreenshotDeviceTest.java index c365dc9..84c0b3b 100644 --- a/android/app/src/androidTest/java/com/saphid/framecontrol/ScreenshotDeviceTest.java +++ b/android/app/src/androidTest/java/com/saphid/framecontrol/ScreenshotDeviceTest.java @@ -9,6 +9,9 @@ import android.database.Cursor; import android.graphics.*; import android.net.Uri; import android.provider.MediaStore; +import android.system.ErrnoException; +import android.system.Os; +import android.system.OsConstants; import android.view.View; import android.view.ViewGroup; import android.widget.Button; @@ -18,6 +21,8 @@ import java.io.*; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.*; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import org.junit.*; import org.junit.runner.RunWith; @@ -37,6 +42,11 @@ public class ScreenshotDeviceTest { for (Uri uri : cleanup) context.getContentResolver().delete(uri, null, null); } + /** Fixture Steam accounts exist on the simulated headset from the first poll. */ + private static SyncEngine.Listing listing(SyncEngine.Shot... shots) { + return new SyncEngine.Listing(Set.of("123", "999999999991"), List.of(shots)); + } + private String name(String headset, SyncEngine.Shot shot) throws Exception { String file = shot.id.substring(shot.id.lastIndexOf('/') + 1); int dot = file.lastIndexOf('.'); @@ -98,12 +108,12 @@ public class ScreenshotDeviceTest { }; GalleryExporter exporter = new GalleryExporter(context, headset, gate); SyncEngine engine = new SyncEngine(root, true); - engine.reconcile(Collections.emptyList(), transport, exporter); + engine.reconcile(listing(), transport, exporter); SyncEngine.Shot shot = new SyncEngine.Shot("999999999991/108/20261024000000_1.png", png.length, 1792800000); - engine.reconcile(List.of(shot), transport, exporter); + engine.reconcile(listing(shot), transport, exporter); assertNull(row(headset, shot)); - engine.reconcile(List.of(shot), transport, exporter); + engine.reconcile(listing(shot), transport, exporter); Uri saved = row(headset, shot); assertNotNull(saved); try (InputStream in = context.getContentResolver().openInputStream(saved)) { @@ -111,16 +121,16 @@ public class ScreenshotDeviceTest { } assertEquals("saved", engine.items().get(0).destination); engine = new SyncEngine(root, true); - engine.reconcile(List.of(shot), transport, exporter); + engine.reconcile(listing(shot), transport, exporter); assertEquals(saved, row(headset, shot)); engine.setGallery(false); SyncEngine.Shot second = new SyncEngine.Shot("999999999991/108/20261024000001_1.png", png.length, 1792800001); - engine.reconcile(List.of(shot, second), transport, exporter); - engine.reconcile(List.of(shot, second), transport, exporter); + engine.reconcile(listing(shot, second), transport, exporter); + engine.reconcile(listing(shot, second), transport, exporter); engine.setGallery(true); engine = new SyncEngine(root, true); - engine.reconcile(List.of(shot, second), transport, exporter); + engine.reconcile(listing(shot, second), transport, exporter); assertNull(row(headset, second)); assertTrue( engine.items().stream() @@ -162,6 +172,141 @@ public class ScreenshotDeviceTest { file.delete(); } + /** + * Stop/restart overlap: a stopped session's exporter is still writing its pending row when the + * next session exports the same screenshot. The old writer reads its original from a FIFO, so + * it stays mid-write, with the row open, until the test supplies the bytes. Without the + * process-wide export lock the next session publishes that row first and the old session's + * cancellation then deletes it, leaving a "saved" receipt with no Gallery image. + */ + @Test + public void stoppedSessionExportCannotDeleteNextSessionsGalleryImage() throws Exception { + String headset = "handoff-" + UUID.randomUUID(); + Bitmap bitmap = Bitmap.createBitmap(16, 16, Bitmap.Config.ARGB_8888); + bitmap.eraseColor(Color.rgb(184, 25, 143)); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + bitmap.compress(Bitmap.CompressFormat.PNG, 100, bytes); + bitmap.recycle(); + byte[] png = bytes.toByteArray(); + SyncEngine.Shot shot = + new SyncEngine.Shot("999999999993/108/20261026000000_1.png", png.length, 1792900000); + File original = File.createTempFile("handoff", ".png", context.getCacheDir()); + Files.write(original.toPath(), png); + File fifo = new File(context.getCacheDir(), "handoff-" + UUID.randomUUID() + ".fifo"); + Os.mkfifo(fifo.getPath(), 0600); + // Both sessions share one settings lock, as both use SyncService.SETTINGS_LOCK. + Object settings = new Object(); + GalleryExporter stopped = new GalleryExporter(context, headset, settings); + GalleryExporter next = new GalleryExporter(context, headset, settings); + AtomicBoolean stoppedSessionCurrent = new AtomicBoolean(true); + AtomicReference stoppedResult = new AtomicReference<>(); + AtomicReference nextResult = new AtomicReference<>(); + Thread old = + exporting(stoppedResult, () -> stopped.export(shot, fifo, stoppedSessionCurrent::get)); + Thread successor = null; + FileDescriptor writer = null; + try { + old.start(); + writer = openWriter(fifo, old); + // The old export has inserted its pending row and waits for the original's bytes. + stoppedSessionCurrent.set(false); // Stop closed the old engine. + successor = exporting(nextResult, () -> next.export(shot, original, () -> true)); + successor.start(); + Thread waiting = successor; + // Fixed: the successor queues behind the old export. Former code: it publishes and ends. + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(20); + while (waiting.isAlive() && !GalleryExporter.EXPORTS.hasQueuedThread(waiting)) { + assertTrue("The next session neither waited nor finished", System.nanoTime() < deadline); + Thread.sleep(10); + } + int written = Os.write(writer, png, 0, png.length); + assertEquals(png.length, written); + } finally { + // An exporter blocked opening the FIFO is not interruptible; a writer's EOF releases it. + if (writer == null) { + try { + writer = Os.open(fifo.getPath(), OsConstants.O_WRONLY | OsConstants.O_NONBLOCK, 0); + } catch (ErrnoException ignored) { + // No reader is waiting. + } + } + if (writer != null) Os.close(writer); + join(old); + if (successor != null) join(successor); + fifo.delete(); + original.delete(); + try { + row(headset, shot); // Registers whatever row a failed run left for cleanup. + } catch (Throwable ignored) { + // Cleanup only; the assertions below report the outcome. + } + } + assertEquals("The stopped session must not save", Boolean.FALSE, result(stoppedResult)); + assertEquals("The next session must save", Boolean.TRUE, result(nextResult)); + Uri saved = row(headset, shot); + assertNotNull("The next session's Gallery image must survive the old callback", saved); + @SuppressWarnings("deprecation") + Uri includingPending = MediaStore.setIncludePending(saved); + try (Cursor c = + context + .getContentResolver() + .query( + includingPending, + new String[] {MediaStore.Images.Media.IS_PENDING}, + null, + null, + null)) { + assertTrue(c != null && c.moveToFirst()); + assertEquals("The Gallery image must be published", 0, c.getInt(0)); + } + try (InputStream in = context.getContentResolver().openInputStream(saved)) { + assertArrayEquals(png, read(in)); + } + } + + private interface Export { + boolean run() throws Exception; + } + + private static Thread exporting(AtomicReference result, Export export) { + return new Thread( + () -> { + try { + result.set(export.run()); + } catch (Throwable e) { + result.set(e); + } + }, + "handoff-export"); + } + + /** Opens the FIFO's write end once the exporter has opened its read end; bounded. */ + private static FileDescriptor openWriter(File fifo, Thread reader) throws Exception { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(20); + while (true) { + try { + return Os.open(fifo.getPath(), OsConstants.O_WRONLY | OsConstants.O_NONBLOCK, 0); + } catch (ErrnoException e) { + if (e.errno != OsConstants.ENXIO) throw e; // ENXIO: no reader yet. + } + assertTrue("The old export ended before reading its original", reader.isAlive()); + assertTrue("The old export never started reading", System.nanoTime() < deadline); + Thread.sleep(10); + } + } + + private static void join(Thread thread) throws InterruptedException { + thread.join(TimeUnit.SECONDS.toMillis(60)); + if (thread.isAlive()) thread.interrupt(); + assertFalse("Export thread did not finish", thread.isAlive()); + } + + private static Object result(AtomicReference result) { + Object value = result.get(); + if (value instanceof Throwable) throw new AssertionError("Export failed", (Throwable) value); + return value; + } + @Test public void galleryOptOutLatchesPreviouslyPairedLibraries() throws Exception { assertFalse("Instrumentation must own a stopped product", SyncService.running()); @@ -194,11 +339,23 @@ public class ScreenshotDeviceTest { SyncEngine.Shot shot = new SyncEngine.Shot("123/108/20261024120000_1.png", 4, 100); File a = SyncService.library(isolated, "headset-a"); File b = SyncService.library(isolated, "headset-b"); + // A retained library still in the version 1 format (no account set), never polled since. + File legacy = SyncService.library(isolated, "headset-legacy"); + assertTrue(legacy.mkdirs()); + Files.write( + new File(legacy, "receipts.json").toPath(), + ("{\"version\":1,\"initialized\":true,\"gallery\":true,\"records\":{\"" + + shot.id + + "\":{\"shot\":{\"id\":\"" + + shot.id + + "\",\"size\":4,\"time\":100},\"state\":\"queued\",\"observations\":2," + + "\"destination\":\"pending\"}}}") + .getBytes(StandardCharsets.UTF_8)); for (File root : List.of(a, b)) { SyncEngine engine = new SyncEngine(root, true); - engine.reconcile(Collections.emptyList(), transfer, unavailable); - engine.reconcile(List.of(shot), transfer, unavailable); - engine.reconcile(List.of(shot), transfer, unavailable); + engine.reconcile(listing(), transfer, unavailable); + engine.reconcile(listing(shot), transfer, unavailable); + engine.reconcile(listing(shot), transfer, unavailable); assertEquals("pending", engine.items().get(0).destination); } assertTrue( @@ -208,12 +365,15 @@ public class ScreenshotDeviceTest { .putBoolean("gallery", true) .commit()); SyncService.gallery(isolated, false); + // The latched legacy journal must stay readable immediately, by the screen and an engine. + assertEquals("appOnly", SyncEngine.snapshot(legacy).get(0).destination); + assertEquals("appOnly", new SyncEngine(legacy, false).items().get(0).destination); SyncService.gallery(isolated, true); - for (File root : List.of(a, b)) { + for (File root : List.of(a, b, legacy)) { SyncEngine engine = new SyncEngine(root, true); assertEquals("appOnly", engine.items().get(0).destination); engine.reconcile( - List.of(shot), + listing(shot), transfer, (s, f, eligible) -> { fail("Opted-out items must never be exported after re-pairing"); @@ -265,9 +425,9 @@ public class ScreenshotDeviceTest { } }; SyncEngine engine = new SyncEngine(root, false); - engine.reconcile(Collections.emptyList(), transfer, (s, f, e) -> false); - engine.reconcile(List.of(shot), transfer, (s, f, e) -> false); - engine.reconcile(List.of(shot), transfer, (s, f, e) -> false); + engine.reconcile(listing(), transfer, (s, f, e) -> false); + engine.reconcile(listing(shot), transfer, (s, f, e) -> false); + engine.reconcile(listing(shot), transfer, (s, f, e) -> false); } assertTrue(prefs.edit().putString("pin", aPin).putBoolean("gallery", false).commit()); activity = @@ -275,33 +435,32 @@ public class ScreenshotDeviceTest { new Intent(context, MainActivity.class) .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK | Intent.FLAG_ACTIVITY_CLEAR_TASK)); instrumentation.addMonitor(monitor); - instrumentation.waitForIdleSync(); Activity screen = activity; - Runnable share = - () -> { - Button button = shareButton(screen.findViewById(android.R.id.content)); - assertNotNull("The downloaded original must be reachable", button); - assertTrue(button.performClick()); - }; - instrumentation.runOnMainSync(share); - assertNotNull(shared.get()); - Uri first = shared.get().getParcelableExtra(Intent.EXTRA_STREAM); + // The library loads on a background executor, so wait for it to render, not for idle. + Uri first = shareWhenRendered(instrumentation, screen, shared, null); try (InputStream in = context.getContentResolver().openInputStream(first)) { assertArrayEquals(new byte[] {1, 2, 3, 4}, read(in)); } assertTrue(prefs.edit().putString("pin", bPin).commit()); - long deadline = System.nanoTime() + java.util.concurrent.TimeUnit.SECONDS.toNanos(5); - Uri selected; - do { - instrumentation.runOnMainSync(share); - selected = shared.get().getParcelableExtra(Intent.EXTRA_STREAM); - if (!first.equals(selected)) break; - Thread.sleep(100); - } while (System.nanoTime() < deadline); - assertNotEquals("Share must no longer target the previous headset", first, selected); + // Until the screen notices the new pin it may still share A; it then clears A's list + // (no button while B loads) and shows only B's original. + Uri selected = shareWhenRendered(instrumentation, screen, shared, first); try (InputStream in = context.getContentResolver().openInputStream(selected)) { assertArrayEquals(new byte[] {5, 6, 7, 8}, read(in)); } + // Once B is shown, A's list is gone: one button, and sharing again still targets B. + AtomicReference buttons = new AtomicReference<>(); + shared.set(null); + instrumentation.runOnMainSync( + () -> { + View content = screen.findViewById(android.R.id.content); + buttons.set(shareButtons(content)); + Button button = shareButton(content); + if (button != null) assertTrue(button.performClick()); + }); + assertEquals("The previous headset's original must be gone", Integer.valueOf(1), buttons.get()); + assertNotNull(shared.get()); + assertEquals(selected, shared.get().getParcelableExtra(Intent.EXTRA_STREAM)); } finally { instrumentation.removeMonitor(monitor); Activity screen = activity; @@ -321,6 +480,48 @@ public class ScreenshotDeviceTest { } } + /** + * Clicks the share button once the asynchronously loaded library shows one and the chosen + * original differs from {@code previous}. A missing button is the expected loading state. + */ + private static Uri shareWhenRendered( + Instrumentation instrumentation, + Activity screen, + AtomicReference shared, + Uri previous) + throws InterruptedException { + long deadline = System.nanoTime() + java.util.concurrent.TimeUnit.SECONDS.toNanos(15); + do { + shared.set(null); + instrumentation.runOnMainSync( + () -> { + Button button = shareButton(screen.findViewById(android.R.id.content)); + if (button != null) assertTrue(button.performClick()); + }); + Intent sent = shared.get(); + if (sent != null) { + Uri uri = sent.getParcelableExtra(Intent.EXTRA_STREAM); + assertNotNull(uri); + if (!uri.equals(previous)) return uri; + } + // Re-check after the screen's next refresh or library load; bounded by the deadline. + Thread.sleep(50); + } while (System.nanoTime() < deadline); + fail("The selected headset's original never became shareable"); + return null; + } + + private static int shareButtons(View view) { + if (view instanceof Button && "Open / share original".contentEquals(((Button) view).getText())) + return 1; + int count = 0; + if (view instanceof ViewGroup) { + ViewGroup group = (ViewGroup) view; + for (int i = 0; i < group.getChildCount(); i++) count += shareButtons(group.getChildAt(i)); + } + return count; + } + private static Button shareButton(View view) { if (view instanceof Button && "Open / share original".contentEquals(((Button) view).getText())) return (Button) view; @@ -334,6 +535,85 @@ public class ScreenshotDeviceTest { return null; } + private Uri insert(String name, boolean pending, byte[] bytes) throws Exception { + ContentResolver resolver = context.getContentResolver(); + ContentValues values = new ContentValues(); + values.put(MediaStore.Images.Media.DISPLAY_NAME, name); + values.put(MediaStore.Images.Media.RELATIVE_PATH, GalleryExporter.FOLDER); + values.put(MediaStore.Images.Media.MIME_TYPE, "image/png"); + values.put(MediaStore.Images.Media.IS_PENDING, 1); + Uri uri = + resolver.insert( + MediaStore.Images.Media.getContentUri(MediaStore.VOLUME_EXTERNAL_PRIMARY), values); + assertNotNull(uri); + cleanup.add(uri); + try (OutputStream out = resolver.openOutputStream(uri, "wt")) { + out.write(bytes); + } + if (!pending) { + ContentValues done = new ContentValues(); + done.put(MediaStore.Images.Media.IS_PENDING, 0); + assertEquals(1, resolver.update(uri, done, null, null)); + } + return uri; + } + + private boolean exists(Uri uri) { + @SuppressWarnings("deprecation") + Uri includingPending = MediaStore.setIncludePending(uri); + try (Cursor c = + context.getContentResolver().query(includingPending, new String[] {"_id"}, null, null, null)) { + return c != null && c.moveToFirst(); + } + } + + @Test + public void optOutDiscardsInterruptedPendingRowsButKeepsPublishedImages() throws Exception { + assertFalse(SyncService.running()); + File files = Files.createTempDirectory(context.getCacheDir().toPath(), "pending-").toFile(); + String preferences = "pending-test-" + UUID.randomUUID(); + Context isolated = + new ContextWrapper(context) { + @Override + public File getFilesDir() { + return files; + } + + @Override + public SharedPreferences getSharedPreferences(String name, int mode) { + return context.getSharedPreferences(preferences, mode); + } + }; + try { + Bitmap bitmap = Bitmap.createBitmap(8, 8, Bitmap.Config.ARGB_8888); + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + bitmap.compress(Bitmap.CompressFormat.PNG, 100, bytes); + bitmap.recycle(); + // A pending row with app-only receipts, from a retained (not currently paired) library. + SyncEngine.Shot shot = new SyncEngine.Shot("123/108/20261024120003_1.png", 4, 100); + SyncEngine engine = new SyncEngine(SyncService.library(isolated, "retained"), true); + engine.reconcile(listing(), null, null); + Uri interrupted = insert(name("retained", shot), true, bytes.toByteArray()); + Uri published = insert("published-" + UUID.randomUUID() + ".png", false, bytes.toByteArray()); + assertTrue(SyncService.prefs(isolated).edit().putBoolean("gallery", true).commit()); + SyncService.gallery(isolated, false); + assertFalse("Hidden pending row must not outlive the opt-out", exists(interrupted)); + // Proven deleted by the product; deleting it again throws SecurityException on API 30+. + cleanup.remove(interrupted); + assertTrue("A published Gallery image must stay", exists(published)); + SyncService.gallery(isolated, true); + assertTrue(exists(published)); + assertFalse(exists(interrupted)); + } finally { + SyncService.prefs(isolated).edit().clear().commit(); + try (java.util.stream.Stream paths = Files.walk(files.toPath())) { + for (java.nio.file.Path path : + paths.sorted(Comparator.reverseOrder()).collect(java.util.stream.Collectors.toList())) + Files.delete(path); + } + } + } + /** Optional real-headset provisioning. Only this test APK exposes the hook. */ @Test public void provisionRealHeadsetIfRequested() throws Exception { @@ -367,12 +647,9 @@ public class ScreenshotDeviceTest { Assume.assumeNotNull(id); String pin = SyncService.prefs(context).getString("pin", null); assertNotNull(pin); - SyncEngine engine = - new SyncEngine( - SyncService.library(context, pin), - SyncService.prefs(context).getBoolean("gallery", true)); + // Read-only: verification must not rewrite the product's journal. SyncEngine.Item item = - engine.items().stream().filter(i -> i.shot.id.equals(id)).findFirst().orElseThrow(); + SyncEngine.snapshot(SyncService.library(context, pin)).stream().filter(i -> i.shot.id.equals(id)).findFirst().orElseThrow(); assertTrue(item.file.exists()); String expected = args.getString("sha256"); assertNotNull(expected); diff --git a/android/app/src/main/java/com/saphid/framecontrol/FrameConnection.java b/android/app/src/main/java/com/saphid/framecontrol/FrameConnection.java index 1895091..3297e7f 100644 --- a/android/app/src/main/java/com/saphid/framecontrol/FrameConnection.java +++ b/android/app/src/main/java/com/saphid/framecontrol/FrameConnection.java @@ -156,16 +156,7 @@ final class FrameConnection implements AutoCloseable, SyncEngine.Transport { }); try { s.connect(15000); - String key = new PhoneIdentity().authorizedKey(install); - run( - s, - "umask 077; mkdir -p ~/.ssh && chmod 700 ~/.ssh && touch ~/.ssh/authorized_keys && chmod" - + " 600 ~/.ssh/authorized_keys && (grep -qxF " - + quote(key) - + " ~/.ssh/authorized_keys || printf '%s\\n' " - + quote(key) - + " >> ~/.ssh/authorized_keys)", - 10000); + run(s, authorizeCommand(new PhoneIdentity().authorizedKey(install)), 10000); } finally { s.disconnect(); } @@ -181,6 +172,56 @@ final class FrameConnection implements AutoCloseable, SyncEngine.Transport { } } + /** + * Idempotently appends {@code key}. A last line without a newline is terminated first, so the + * existing key is never merged with the new one (the same guard as the iPhone app). + */ + static String authorizeCommand(String key) { + String file = "~/.ssh/authorized_keys"; + return "umask 077; mkdir -p ~/.ssh && chmod 700 ~/.ssh && touch " + + file + + " && chmod 600 " + + file + + " && { grep -qxF " + + quote(key) + + " " + + file + + " || { if [ -s " + + file + + " ] && [ -n \"$(tail -c 1 " + + file + + ")\" ]; then printf '\\n' >> " + + file + + "; fi; printf '%s\\n' " + + quote(key) + + " >> " + + file + + "; }; }"; + } + + /** + * Lists Steam account directories, including ones with no screenshots. Prints "absent" when + * Steam has not created userdata yet; any other failure exits non-zero. + */ + static final String ACCOUNTS_COMMAND = + "d=\"$HOME/.local/share/Steam/userdata\"; if [ ! -e \"$d\" ]; then echo absent; else" + + " cd \"$d\" || exit 1; echo present; for a in *; do case \"$a\" in ''|*[!0-9]*) ;;" + + " *) if [ -d \"$a\" ]; then printf '%s\\n' \"$a\"; fi ;; esac; done; fi"; + + static Set parseAccounts(String output) throws IOException { + String[] lines = output.split("\n", -1); + Set accounts = new TreeSet<>(); + if (lines.length == 2 && lines[0].equals("absent") && lines[1].isEmpty()) return accounts; + if (lines.length < 2 || !lines[0].equals("present") || !lines[lines.length - 1].isEmpty()) + throw new IOException("Could not read Steam accounts on the headset."); + for (int i = 1; i < lines.length - 1; i++) { + if (!lines[i].matches("[0-9]+")) throw new IOException("Unexpected Steam account entry."); + // The headset server ignores longer ids; so does the screenshot listing. + if (lines[i].length() <= 12) accounts.add(lines[i]); + } + return accounts; + } + static String run(Session s, String command, int timeout) throws Exception { ChannelExec channel = (ChannelExec) s.openChannel("exec"); channel.setCommand(command); @@ -332,6 +373,12 @@ final class FrameConnection implements AutoCloseable, SyncEngine.Transport { return c; } + /** Accounts are read first; one that appears before the screenshot query is still new. */ + SyncEngine.Listing listing() throws Exception { + Set accounts = parseAccounts(run(session, ACCOUNTS_COMMAND, 10000)); + return new SyncEngine.Listing(accounts, shots()); + } + List shots() throws Exception { HttpURLConnection c = request("/api/shots"); try (InputStream in = c.getInputStream()) { diff --git a/android/app/src/main/java/com/saphid/framecontrol/GalleryExporter.java b/android/app/src/main/java/com/saphid/framecontrol/GalleryExporter.java index b3d6e0f..14bbe9f 100644 --- a/android/app/src/main/java/com/saphid/framecontrol/GalleryExporter.java +++ b/android/app/src/main/java/com/saphid/framecontrol/GalleryExporter.java @@ -5,10 +5,26 @@ import android.database.Cursor; import android.net.Uri; import android.provider.MediaStore; import java.io.*; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.BooleanSupplier; /** Deterministic, app-owned MediaStore rows make interrupted exports retryable. */ final class GalleryExporter implements SyncEngine.Exporter { + static final String FOLDER = "Pictures/Frame Control/"; + + /** + * Serializes every export in the process, across exporter instances and service sessions. A + * stopped session's exporter may still be writing when a new session starts; without this, the + * new one could publish the same deterministic row and the old one's cancellation would then + * delete it. Lock order is EXPORTS, then the settings lock. Never take EXPORTS while holding + * the settings lock: {@link #discardPending} runs under it and must not wait for exports. + */ + static final ReentrantLock EXPORTS = new ReentrantLock(); + + private static final long EXPORT_WAIT_SECONDS = 30; private final Context context; private final String headset; private final Object settingsLock; @@ -19,9 +35,57 @@ final class GalleryExporter implements SyncEngine.Exporter { this.settingsLock = settingsLock; } + /** + * Best-effort removal of this app's hidden pending rows in the Frame Control folder. Only an + * interrupted export leaves one; published (non-pending) images are never touched. Callers + * hold the settings lock, so no export can be publishing a row at the same time. + */ + static int discardPending(Context context) { + ContentResolver resolver = context.getContentResolver(); + Uri collection = MediaStore.Images.Media.getContentUri(MediaStore.VOLUME_EXTERNAL_PRIMARY); + @SuppressWarnings("deprecation") + Uri includingPending = MediaStore.setIncludePending(collection); + List rows = new ArrayList<>(); + int removed = 0; + try { + try (Cursor cursor = + resolver.query( + includingPending, + new String[] {"_id"}, + MediaStore.Images.Media.IS_PENDING + + "=1 AND " + + MediaStore.Images.Media.RELATIVE_PATH + + "=? AND " + + MediaStore.Images.Media.OWNER_PACKAGE_NAME + + "=?", + new String[] {FOLDER, context.getPackageName()}, + null)) { + while (cursor != null && cursor.moveToNext()) + rows.add(ContentUris.withAppendedId(collection, cursor.getLong(0))); + } + for (Uri row : rows) removed += resolver.delete(row, null, null); + } catch (RuntimeException ignored) { + // Android 11+ also expires abandoned pending rows; never fail the setting change here. + } + return removed; + } + @Override public boolean export(SyncEngine.Shot shot, File file, BooleanSupplier eligible) throws Exception { + // Interruptible and bounded: Stop still ends a waiting session, and a stuck earlier write + // only delays this item to a later poll. + if (!EXPORTS.tryLock(EXPORT_WAIT_SECONDS, TimeUnit.SECONDS)) + throw new IOException("Gallery is still finishing an earlier save."); + try { + return exportLocked(shot, file, eligible); + } finally { + EXPORTS.unlock(); + } + } + + private boolean exportLocked(SyncEngine.Shot shot, File file, BooleanSupplier eligible) + throws Exception { String name = shot.id.substring(shot.id.lastIndexOf('/') + 1); int dot = name.lastIndexOf('.'); name = @@ -29,7 +93,7 @@ final class GalleryExporter implements SyncEngine.Exporter { + "_" + FrameConnection.hash(headset + "/" + shot.id).substring(0, 20) + name.substring(dot); - String folder = "Pictures/Frame Control/"; + String folder = FOLDER; ContentResolver resolver = context.getContentResolver(); Uri collection = MediaStore.Images.Media.getContentUri(MediaStore.VOLUME_EXTERNAL_PRIMARY); // API 29 excludes even app-owned pending rows unless explicitly included. @@ -78,6 +142,7 @@ final class GalleryExporter implements SyncEngine.Exporter { } synchronized (settingsLock) { if (!eligible.getAsBoolean()) { + // Holding EXPORTS, no other exporter can have published this row since we found it. resolver.delete(row, null, null); return false; } diff --git a/android/app/src/main/java/com/saphid/framecontrol/MainActivity.java b/android/app/src/main/java/com/saphid/framecontrol/MainActivity.java index aa7a893..6f62b7b 100644 --- a/android/app/src/main/java/com/saphid/framecontrol/MainActivity.java +++ b/android/app/src/main/java/com/saphid/framecontrol/MainActivity.java @@ -17,6 +17,10 @@ import java.util.concurrent.*; public final class MainActivity extends Activity { private final ExecutorService network = Executors.newSingleThreadExecutor(); + // Journal parsing and file checks stay off the main thread. + private final ExecutorService library = Executors.newSingleThreadExecutor(); + private boolean loading; + private String renderedPin; private final Handler handler = new Handler(Looper.getMainLooper()); private LinearLayout root, downloads; private FrameLayout content; @@ -33,6 +37,9 @@ public final class MainActivity extends Activity { private static final int INK = Color.rgb(232, 240, 238), MUTED = Color.rgb(164, 183, 179), ACCENT = Color.rgb(124, 216, 195); + private static final String EMPTY = + "Pair and start syncing. Existing headset screenshots stay on the headset; new captures" + + " will appear here."; private final Runnable refresh = new Runnable() { public void run() { @@ -169,8 +176,8 @@ public final class MainActivity extends Activity { companionBody.addView( text( "New captures download automatically while sync is running. A notification keeps the" - + " connection visible. Stop syncing to disconnect. Android may delay transfers in" - + " battery-saving modes.", + + " connection visible. Stop syncing to disconnect. Android may delay transfers" + + " while the phone is locked or in battery-saving modes.", 14, MUTED)); companionBody.addView( @@ -179,12 +186,7 @@ public final class MainActivity extends Activity { + " earlier app-only downloads.", 14, MUTED)); - empty = - text( - "Pair and start syncing. Existing headset screenshots stay on the headset; new captures" - + " will appear here.", - 14, - MUTED); + empty = text(EMPTY, 14, MUTED); companionBody.addView(empty); downloads = new LinearLayout(this); downloads.setOrientation(LinearLayout.VERTICAL); @@ -297,51 +299,89 @@ public final class MainActivity extends Activity { port.setEnabled(!running && !busy); user.setEnabled(!running && !busy); if (controls && !Objects.equals(loadedPage, SyncService.page())) show(true); - try { - List items = SyncService.items(this); - StringBuilder signature = new StringBuilder(); - for (var item : items) - signature - .append(item.file.getAbsolutePath()) - .append('|') - .append(item.destination) - .append('|') - .append(item.file.exists()) - .append('\n'); - if (signature.toString().equals(librarySignature)) return; - librarySignature = signature.toString(); + String pin = SyncService.prefs(this).getString("pin", null); + if (!Objects.equals(pin, renderedPin)) { + // Never leave another headset's originals shareable while the new library loads. + renderedPin = pin; + librarySignature = ""; downloads.removeAllViews(); - empty.setVisibility(items.isEmpty() ? View.VISIBLE : View.GONE); - int shown = 0; - for (SyncEngine.Item item : items) { - if (shown++ >= downloadLimit) { - downloads.addView( - button( - "Show 100 more downloads", - () -> { - downloadLimit += 100; - librarySignature = ""; - update(); - })); - break; - } - String label = item.shot.id.substring(item.shot.id.lastIndexOf('/') + 1); - LinearLayout card = new LinearLayout(this); - card.setOrientation(LinearLayout.VERTICAL); - card.setPadding(0, dp(12), 0, dp(12)); - card.addView(text(label, 16, INK)); - String where = - item.destination.equals("saved") - ? "Saved to Gallery" - : item.destination.equals("appOnly") ? "In Frame Control" : "Gallery save pending"; - if (!item.file.exists()) where = "Download pending"; - card.addView(text(where + " · Game " + item.shot.id.split("/")[1], 13, MUTED)); - if (item.file.exists()) card.addView(button("Open / share original", () -> share(item))); - downloads.addView(card); - } - } catch (Exception e) { + empty.setText(EMPTY); empty.setVisibility(View.VISIBLE); - empty.setText("Could not read screenshot downloads: " + SyncService.message(e)); + } + if (loading) return; + loading = true; + library.execute( + () -> { + List items = null; + Set present = new HashSet<>(); + Exception failure = null; + try { + items = SyncService.items(this, pin); + for (SyncEngine.Item item : items) if (item.file.exists()) present.add(item); + } catch (Exception e) { + failure = e; + } + List loaded = items; + Exception error = failure; + runOnUiThread( + () -> { + loading = false; + if (isFinishing() || isDestroyed()) return; + // A result for a previously paired headset is discarded; the next tick reloads. + if (!Objects.equals(pin, SyncService.prefs(this).getString("pin", null))) return; + if (error != null) { + librarySignature = ""; + empty.setVisibility(View.VISIBLE); + empty.setText("Could not read screenshot downloads: " + SyncService.message(error)); + return; + } + render(loaded, present); + }); + }); + } + + private void render(List items, Set present) { + StringBuilder signature = new StringBuilder(); + for (var item : items) + signature + .append(item.file.getAbsolutePath()) + .append('|') + .append(item.destination) + .append('|') + .append(present.contains(item)) + .append('\n'); + if (signature.toString().equals(librarySignature)) return; + librarySignature = signature.toString(); + downloads.removeAllViews(); + empty.setText(EMPTY); + empty.setVisibility(items.isEmpty() ? View.VISIBLE : View.GONE); + int shown = 0; + for (SyncEngine.Item item : items) { + if (shown++ >= downloadLimit) { + downloads.addView( + button( + "Show 100 more downloads", + () -> { + downloadLimit += 100; + librarySignature = ""; + update(); + })); + break; + } + String label = item.shot.id.substring(item.shot.id.lastIndexOf('/') + 1); + LinearLayout card = new LinearLayout(this); + card.setOrientation(LinearLayout.VERTICAL); + card.setPadding(0, dp(12), 0, dp(12)); + card.addView(text(label, 16, INK)); + String where = + item.destination.equals("saved") + ? "Saved to Gallery" + : item.destination.equals("appOnly") ? "In Frame Control" : "Gallery save pending"; + boolean exists = present.contains(item); + if (!exists) where = "Download pending"; + card.addView(text(where + " · Game " + item.shot.id.split("/")[1], 13, MUTED)); + if (exists) card.addView(button("Open / share original", () -> share(item))); + downloads.addView(card); } } @@ -502,6 +542,7 @@ public final class MainActivity extends Activity { @Override protected void onDestroy() { network.shutdownNow(); + library.shutdownNow(); if (fileSelection != null) fileSelection.onReceiveValue(null); if (web != null) { web.destroy(); diff --git a/android/app/src/main/java/com/saphid/framecontrol/SessionGate.java b/android/app/src/main/java/com/saphid/framecontrol/SessionGate.java new file mode 100644 index 0000000..214fd84 --- /dev/null +++ b/android/app/src/main/java/com/saphid/framecontrol/SessionGate.java @@ -0,0 +1,71 @@ +package com.saphid.framecontrol; + +import java.util.function.Consumer; + +/** + * Owns the visible sync status. Each service instance gets one session token; once a session + * ends, its late worker can no longer change the status, page or notification, even after a + * newer session has started. + */ +final class SessionGate { + static final String DISCONNECTED = "Disconnected"; + private long issued; + private long current; + private String state = DISCONNECTED; + private String page; + + synchronized long begin() { + current = ++issued; + page = null; + return current; + } + + synchronized boolean active(long session) { + return session != 0 && session == current; + } + + synchronized boolean running() { + return current != 0; + } + + synchronized String state() { + return state; + } + + synchronized String page() { + return page; + } + + /** Publishes a status and runs {@code show} (the notification update) only while current. */ + synchronized boolean status(long session, String text, Consumer show) { + if (!active(session)) return false; + state = text; + if (show != null) show.accept(text); + return true; + } + + synchronized boolean page(long session, String value) { + if (!active(session)) return false; + page = value; + return true; + } + + /** Runs {@code action} only while the session is current, serialized against {@link #end}. */ + synchronized boolean ifActive(long session, Runnable action) { + if (!active(session)) return false; + action.run(); + return true; + } + + /** Ends only the named session. Progress text becomes Disconnected; a final error stays. */ + synchronized boolean end(long session) { + if (!active(session)) return false; + current = 0; + page = null; + if (state.startsWith("Syncing") + || state.startsWith("Waiting") + || state.startsWith("Connecting") + || state.startsWith("Screenshot pending")) state = DISCONNECTED; + return true; + } +} diff --git a/android/app/src/main/java/com/saphid/framecontrol/SyncEngine.java b/android/app/src/main/java/com/saphid/framecontrol/SyncEngine.java index fd03669..fdc9d1a 100644 --- a/android/app/src/main/java/com/saphid/framecontrol/SyncEngine.java +++ b/android/app/src/main/java/com/saphid/framecontrol/SyncEngine.java @@ -31,6 +31,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.TreeSet; import java.util.function.BooleanSupplier; import java.util.regex.Pattern; @@ -39,6 +40,7 @@ public final class SyncEngine { private static final long MAX_SIZE = 30L * 1024 * 1024; private static final Pattern ID = Pattern.compile( "^[0-9]{1,12}/[0-9]{1,20}/[0-9]{14}_[0-9]{1,4}\\.(jpg|png)$"); + private static final Pattern ACCOUNT = Pattern.compile("^[0-9]{1,12}$"); private static final Gson GSON = new GsonBuilder().registerTypeAdapter(Shot.class, new TypeAdapter() { @Override public void write(JsonWriter writer, Shot shot) throws IOException { @@ -92,6 +94,20 @@ public final class SyncEngine { } } + /** + * One headset poll. {@code accounts} names every Steam account directory present on the + * headset, including accounts with no screenshots yet. An absent userdata root is an empty set. + */ + public static final class Listing { + public final Set accounts; + public final List shots; + + public Listing(Set accounts, List shots) { + this.accounts = accounts; + this.shots = shots; + } + } + public interface Transport { void download(Shot shot, File destination) throws Exception; boolean validImage(File f) throws Exception; @@ -119,9 +135,17 @@ public final class SyncEngine { Integer version; Boolean initialized; Boolean gallery; + /** Accounts already baselined. Absent only in version 1 journals. */ + Set accounts; Map records; } + private static final class Poll { + final Map eligible = new LinkedHashMap<>(); + final Set accounts = new TreeSet<>(); + int skipped; + } + private static final class Receipt { Shot shot; String state; @@ -152,18 +176,29 @@ public final class SyncEngine { private Ledger ledger; private long generation; private long active; + private boolean closed; + private volatile int skipped; public SyncEngine(File root, boolean gallery) throws Exception { + this(root, Boolean.valueOf(gallery)); + } + + /** A null gallery opens a read-only view that never creates, upgrades or rewrites anything. */ + private SyncEngine(File root, Boolean gallery) throws Exception { if (root == null || Files.isSymbolicLink(root.toPath())) { throw new IOException("Unsafe sync root"); } this.root = root.getCanonicalFile().toPath(); - List missingDirectories = new ArrayList<>(); - for (Path path = this.root; path != null && !Files.exists(path); path = path.getParent()) { - missingDirectories.add(path); + if (gallery == null) { + closed = true; + } else { + List missingDirectories = new ArrayList<>(); + for (Path path = this.root; path != null && !Files.exists(path); path = path.getParent()) { + missingDirectories.add(path); + } + Files.createDirectories(this.root); + for (Path directory : missingDirectories) syncDirectory(directory.getParent()); } - Files.createDirectories(this.root); - for (Path directory : missingDirectories) syncDirectory(directory.getParent()); originals = this.root.resolve("originals"); journal = this.root.resolve("receipts.json"); checkPath(originals); @@ -188,7 +223,7 @@ public final class SyncEngine { throw new IOException("Corrupt sync journal", e); } validateLedger(); - setGallery(gallery); + if (gallery != null) setGallery(gallery); } else { if (Files.exists(originals, LinkOption.NOFOLLOW_LINKS)) { try (java.util.stream.Stream paths = Files.walk(originals)) { @@ -198,14 +233,25 @@ public final class SyncEngine { } } ledger = emptyLedger(); - ledger.gallery = gallery; - persist(ledger); + if (gallery == null) { + ledger.gallery = false; + } else { + ledger.gallery = gallery; + persist(ledger); + } } } - public void setGallery(boolean enabled) throws Exception { + /** Post-baseline receipts read without locks, directory creation or journal writes. */ + public static List snapshot(File root) throws Exception { + return new SyncEngine(root, (Boolean) null).items(); + } + + /** Returns false, without writing, once this engine is closed. */ + public boolean setGallery(boolean enabled) throws Exception { synchronized (lock) { - if (ledger.gallery == enabled) return; + if (closed) return false; + if (ledger.gallery == enabled) return true; Ledger next = copy(); next.gallery = enabled; if (!enabled) { @@ -214,53 +260,87 @@ public final class SyncEngine { } } commit(next); + return true; } } - /** A second live reconcile is rejected. stop() cancels its generation and permits a new run. */ - public void reconcile(List shots, Transport transport, Exporter exporter) throws Exception { + /** + * A second live reconcile is rejected. stop() cancels its generation and permits a new run; + * close() cancels it permanently. The journal is rewritten only when a receipt changes. + */ + public void reconcile(Listing listing, Transport transport, Exporter exporter) throws Exception { final long run; synchronized (lock) { + if (closed) throw new IllegalStateException("Sync stopped"); if (active != 0) throw new IllegalStateException("Sync already in progress"); run = ++generation; active = run; } try { - Map listing = listing(shots); + Poll poll = poll(listing); synchronized (lock) { if (active != run) return; + skipped = poll.skipped; Ledger next = copy(); if (!next.initialized) { - for (Shot shot : listing.values()) { + for (Shot shot : poll.eligible.values()) { next.records.put(shot.id, new Receipt(shot, "baseline", 0, null)); } next.initialized = true; + next.version = 2; + next.accounts = poll.accounts; commit(next); return; } + boolean changed = false; + Set known = next.accounts; + if (known == null) { + // Version 1 journals did not record accounts. Trust only accounts that already + // have receipts; any other account present now is history and is baselined. + known = new TreeSet<>(); + for (String id : next.records.keySet()) known.add(account(id)); + // The account comparison below always records the set with this version. + next.version = 2; + changed = true; + } // Missing observations break consecutiveness, including across restarts. for (Receipt receipt : next.records.values()) { - if (!listing.containsKey(receipt.shot.id) + if (!poll.eligible.containsKey(receipt.shot.id) && receipt.observations != 0 && ("observing".equals(receipt.state) || "queued".equals(receipt.state))) { receipt.observations = 0; + changed = true; } } - for (Shot shot : listing.values()) { + for (Shot shot : poll.eligible.values()) { Receipt receipt = next.records.get(shot.id); - if (receipt == null) { + if (receipt == null && !known.contains(account(shot.id))) { + // A newly appearing account (or a returning one) starts with its own + // baseline, so its existing screenshots are never imported. + next.records.put(shot.id, new Receipt(shot, "baseline", 0, null)); + changed = true; + } else if (receipt == null) { // Latch eligibility at discovery, before waiting for a stable second sample. next.records.put(shot.id, new Receipt(shot, "observing", 1, next.gallery ? "pending" : "appOnly")); + changed = true; } else if ("observing".equals(receipt.state) || "queued".equals(receipt.state)) { - receipt.observations = same(receipt.shot, shot) - ? Math.min(2, receipt.observations + 1) : 1; + boolean stable = same(receipt.shot, shot); + int observations = stable ? Math.min(2, receipt.observations + 1) : 1; + if (!stable || observations != receipt.observations) changed = true; + receipt.observations = observations; receipt.shot = shot; if (receipt.observations == 2 && "observing".equals(receipt.state)) { receipt.state = "queued"; } } } - commit(next); // Queue and its disposition reach disk before any download. + // Accounts absent now are forgotten; if one returns it is baselined again. + if (!poll.accounts.equals(next.accounts)) { + next.accounts = poll.accounts; + changed = true; + } + // Queue and its disposition reach disk before any download. + if (changed) commit(next); } List ids; @@ -317,6 +397,11 @@ public final class SyncEngine { } } + /** Listed files left out of the last poll because they were empty or over 30 MiB. */ + public int skipped() { + return skipped; + } + public void stop() { synchronized (lock) { ++generation; @@ -324,6 +409,24 @@ public final class SyncEngine { } } + /** + * Permanently cancels this engine. Once it returns, no in-flight or later call can write the + * journal, so another engine may safely take over the same root. + */ + public void close() { + synchronized (lock) { + closed = true; + ++generation; + active = 0; + } + } + + public boolean isClosed() { + synchronized (lock) { + return closed; + } + } + private void download(long run, Shot shot, Transport transport) throws Exception { Path temporary; synchronized (lock) { @@ -411,36 +514,79 @@ public final class SyncEngine { && "downloaded".equals(receipt.state) && "pending".equals(receipt.destination); } - private static Map listing(List shots) throws IOException { - if (shots == null) throw new IOException("Missing screenshot listing"); - Map result = new LinkedHashMap<>(); - for (Shot shot : shots) { - validateShot(shot); - Shot previous = result.put(shot.id, shot); + private static Poll poll(Listing listing) throws IOException { + if (listing == null || listing.shots == null || listing.accounts == null) { + throw new IOException("Missing screenshot listing"); + } + Poll result = new Poll(); + for (String account : listing.accounts) { + if (account == null || !ACCOUNT.matcher(account).matches()) { + throw new IOException("Invalid Steam account"); + } + result.accounts.add(account); + } + Map seen = new LinkedHashMap<>(); + for (Shot shot : listing.shots) { + // Unsafe ids, non-finite times and conflicting duplicates reject the whole listing. + validateIdentity(shot); + Shot previous = seen.put(shot.id, shot); if (previous != null && !same(previous, shot)) { throw new IOException("Conflicting screenshot identity: " + shot.id); } + // The account directory may appear between the account and screenshot queries. + result.accounts.add(account(shot.id)); + } + for (Shot shot : seen.values()) { + // An empty, still-writing or oversized file is only this item's problem: treat it + // as absent from this poll so it never blocks other screenshots or the baseline. + if (eligibleSize(shot)) result.eligible.put(shot.id, shot); + else result.skipped++; } return result; } - private static void validateShot(Shot shot) throws IOException { + private static String account(String id) { + return id.substring(0, id.indexOf('/')); + } + + private static void validateIdentity(Shot shot) throws IOException { if (shot == null || shot.id == null || !ID.matcher(shot.id).matches() - || shot.size <= 0 || shot.size > MAX_SIZE || !Double.isFinite(shot.time)) { + || !Double.isFinite(shot.time)) { throw new IOException("Invalid screenshot metadata"); } } + private static boolean eligibleSize(Shot shot) { + return shot.size > 0 && shot.size <= MAX_SIZE; + } + + private static void validateShot(Shot shot) throws IOException { + validateIdentity(shot); + if (!eligibleSize(shot)) throw new IOException("Invalid screenshot metadata"); + } + private static boolean same(Shot a, Shot b) { return a.size == b.size && Double.compare(a.time, b.time) == 0; } private void validateLedger() throws IOException { - if (ledger == null || ledger.version == null || ledger.version != 1 - || ledger.initialized == null || ledger.gallery == null || ledger.records == null - || (!ledger.initialized && !ledger.records.isEmpty())) { + if (ledger == null || ledger.version == null || ledger.initialized == null + || ledger.gallery == null || ledger.records == null + || (ledger.version == 1 && ledger.accounts != null) + || (ledger.version == 2 && ledger.accounts == null) + || (ledger.version != 1 && ledger.version != 2) + || (!ledger.initialized && !ledger.records.isEmpty()) + || (!ledger.initialized && ledger.accounts != null && !ledger.accounts.isEmpty())) { throw new IOException("Invalid sync journal header"); } + if (ledger.accounts != null) { + for (String account : ledger.accounts) { + if (account == null || !ACCOUNT.matcher(account).matches()) { + throw new IOException("Invalid sync journal header"); + } + } + ledger.accounts = new TreeSet<>(ledger.accounts); + } for (Map.Entry entry : ledger.records.entrySet()) { Receipt receipt = entry.getValue(); if (receipt == null) throw new IOException("Invalid sync receipt"); @@ -515,10 +661,16 @@ public final class SyncEngine { } } + /** + * Keeps the version: a version 1 journal (no account set) stays version 1 through setting + * changes, and only reconcile upgrades it, recording the version and accounts together. + */ private Ledger copy() { Ledger next = emptyLedger(); + next.version = ledger.version; next.initialized = ledger.initialized; next.gallery = ledger.gallery; + next.accounts = ledger.accounts == null ? null : new TreeSet<>(ledger.accounts); for (Map.Entry entry : ledger.records.entrySet()) { Receipt receipt = entry.getValue(); next.records.put(entry.getKey(), new Receipt(receipt.shot, receipt.state, @@ -529,13 +681,15 @@ public final class SyncEngine { private static Ledger emptyLedger() { Ledger result = new Ledger(); - result.version = 1; + result.version = 2; result.initialized = false; + result.accounts = new TreeSet<>(); result.records = new LinkedHashMap<>(); return result; } private void commit(Ledger next) throws IOException { + if (closed) throw new IOException("Sync journal is read-only"); persist(next); ledger = next; } diff --git a/android/app/src/main/java/com/saphid/framecontrol/SyncService.java b/android/app/src/main/java/com/saphid/framecontrol/SyncService.java index d6f04d7..f51f212 100644 --- a/android/app/src/main/java/com/saphid/framecontrol/SyncService.java +++ b/android/app/src/main/java/com/saphid/framecontrol/SyncService.java @@ -12,29 +12,31 @@ import java.util.concurrent.*; public final class SyncService extends Service { static final String CHANNEL = "headset-sync"; static final Object SETTINGS_LOCK = new Object(); + private static final int NOTIFICATION = 108; + private static final SessionGate GATE = new SessionGate(); private static volatile SyncService instance; - private static volatile String state = "Disconnected"; - private static volatile String page; private final ExecutorService worker = Executors.newSingleThreadExecutor(); + private long session; private volatile boolean stopping; private volatile FrameConnection connection; private volatile SyncEngine engine; + private volatile String pin; static SharedPreferences prefs(Context c) { return c.getSharedPreferences("frame", MODE_PRIVATE); } static String state() { - return state; + return GATE.state(); } static String page() { - return page; + return GATE.page(); } static boolean running() { SyncService service = instance; - return service != null && !service.stopping; + return service != null && !service.stopping && GATE.active(service.session); } static String install(Context c) { @@ -58,7 +60,8 @@ public final class SyncService extends Service { // Journal first: a failed disk write must not appear as a saved setting. String pin = prefs(c).getString("pin", null); SyncEngine live = service == null ? null : service.engine; - if (live != null) live.setGallery(value); + // A closed engine belongs to a stopped session; update its library from disk instead. + if (live != null && !live.setGallery(value)) live = null; if (!value) { // The setting is global. Latch the opt-out in every retained library, // including headsets that are not currently paired or connected. @@ -78,22 +81,24 @@ public final class SyncService extends Service { } if (!prefs(c).edit().putBoolean("gallery", value).commit()) throw new java.io.IOException("Could not save the Gallery setting."); + // Interrupted exports can leave hidden pending rows; app-only receipts never retry them. + if (!value) GalleryExporter.discardPending(c); } } - static List items(Context c) throws Exception { - SyncService service = instance; - if (service != null && service.engine != null) return service.engine.items(); - String pin = prefs(c).getString("pin", null); + /** Read-only, for a background thread. Never creates or rewrites a library. */ + static List items(Context c, String pin) throws Exception { if (pin == null) return Collections.emptyList(); - synchronized (SETTINGS_LOCK) { - return new SyncEngine(library(c, pin), prefs(c).getBoolean("gallery", true)).items(); - } + SyncService service = instance; + SyncEngine live = service == null ? null : service.engine; + if (live != null && !live.isClosed() && pin.equals(service.pin)) return live.items(); + return SyncEngine.snapshot(library(c, pin)); } @Override public void onCreate() { super.onCreate(); + session = GATE.begin(); instance = this; stopping = false; NotificationManager manager = getSystemService(NotificationManager.class); @@ -125,9 +130,12 @@ public final class SyncService extends Service { .build(); } + /** A stale worker cannot reach the notification once its session has ended. */ private void status(String text) { - state = text; - getSystemService(NotificationManager.class).notify(108, notification(text)); + GATE.status( + session, + text, + t -> getSystemService(NotificationManager.class).notify(NOTIFICATION, notification(t))); } @Override @@ -137,11 +145,12 @@ public final class SyncService extends Service { return START_NOT_STICKY; } if (connection != null || stopping) return START_NOT_STICKY; - state = "Connecting to headset"; + String text = "Connecting to headset"; + GATE.status(session, text, null); if (Build.VERSION.SDK_INT >= 34) startForeground( - 108, notification(state), ServiceInfo.FOREGROUND_SERVICE_TYPE_CONNECTED_DEVICE); - else startForeground(108, notification(state)); + NOTIFICATION, notification(text), ServiceInfo.FOREGROUND_SERVICE_TYPE_CONNECTED_DEVICE); + else startForeground(NOTIFICATION, notification(text)); connection = new FrameConnection(this); worker.execute(this::runSession); return START_NOT_STICKY; @@ -152,13 +161,22 @@ public final class SyncService extends Service { SharedPreferences p = prefs(this); String pin = p.getString("pin", null); if (pin == null) throw new java.io.IOException("Pair with your headset first."); + SyncEngine created; synchronized (SETTINGS_LOCK) { - engine = new SyncEngine(library(this, pin), p.getBoolean("gallery", true)); + boolean gallery = p.getBoolean("gallery", true); + created = new SyncEngine(library(this, pin), gallery); + // Retry cleanup of hidden rows if an earlier opt-out's cleanup was interrupted. + if (!gallery) GalleryExporter.discardPending(this); + this.pin = pin; + engine = created; } + // Pairs with onDestroy: it sets stopping before reading engine. + if (stopping) created.close(); while (!stopping) { try { FrameConnection active = new FrameConnection(this); connection = active; + if (stopping) break; status("Connecting to headset"); active.connect( p.getString("host", ""), @@ -170,13 +188,14 @@ public final class SyncService extends Service { active.close(); break; } - page = active.page(); + GATE.page(session, active.page()); status("Syncing new screenshots"); GalleryExporter exporter = new GalleryExporter(this, pin, SETTINGS_LOCK); while (!stopping && active.connected()) { try { - engine.reconcile(active.shots(), active, exporter); - status("Syncing new screenshots"); + SyncEngine.Listing listing = active.listing(); + created.reconcile(listing, active, exporter); + status(progress(listing, created.skipped())); } catch (Exception e) { status("Screenshot pending · " + message(e)); } @@ -184,8 +203,9 @@ public final class SyncService extends Service { } if (!stopping) throw new java.io.IOException("Connection lost. Reconnecting…"); } catch (Exception e) { - page = null; - if (connection != null) connection.close(); + GATE.page(session, null); + FrameConnection c = connection; + if (c != null) c.close(); if (stopping) break; status("Waiting for headset · " + message(e)); Thread.sleep(10000); @@ -194,14 +214,24 @@ public final class SyncService extends Service { } catch (InterruptedException ignored) { Thread.currentThread().interrupt(); } catch (Exception e) { - state = message(e); + GATE.status(session, message(e), null); } finally { - page = null; - if (connection != null) connection.close(); - stopSelf(); + GATE.page(session, null); + FrameConnection c = connection; + if (c != null) c.close(); + // Only a still-current session may stop the service; Stop already ended a stale one. + GATE.ifActive(session, this::stopSelf); } } + static String progress(SyncEngine.Listing listing, int skipped) { + if (listing.accounts.isEmpty()) return "Waiting for a Steam account on the headset"; + String text = "Syncing new screenshots"; + if (skipped == 1) text += " · 1 file skipped (empty or over 30 MB)"; + else if (skipped > 1) text += " · " + skipped + " files skipped (empty or over 30 MB)"; + return text; + } + static String message(Exception e) { String text = e.getMessage(); if (text == null || text.trim().isEmpty()) return "Try reconnecting."; @@ -215,15 +245,17 @@ public final class SyncService extends Service { @Override public void onDestroy() { stopping = true; - page = null; - if (engine != null) engine.stop(); - if (connection != null) connection.close(); + // After this, the old worker cannot post status, page or notification updates. + GATE.end(session); + SyncEngine live = engine; + if (live != null) live.close(); + FrameConnection c = connection; + if (c != null) c.close(); worker.shutdownNow(); - instance = null; - if (state.equals("Syncing new screenshots") - || state.startsWith("Waiting") - || state.startsWith("Connecting")) state = "Disconnected"; + if (instance == this) instance = null; stopForeground(STOP_FOREGROUND_REMOVE); + // Also clear a notification posted just before the gate closed. + getSystemService(NotificationManager.class).cancel(NOTIFICATION); super.onDestroy(); } diff --git a/android/app/src/test/java/com/saphid/framecontrol/FrameConnectionTest.java b/android/app/src/test/java/com/saphid/framecontrol/FrameConnectionTest.java new file mode 100644 index 0000000..a93dcfc --- /dev/null +++ b/android/app/src/test/java/com/saphid/framecontrol/FrameConnectionTest.java @@ -0,0 +1,106 @@ +package com.saphid.framecontrol; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.attribute.PosixFilePermissions; +import java.util.Arrays; +import java.util.Collections; +import java.util.TreeSet; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.*; + +/** Runs the generated headset shell against a temporary HOME. */ +public class FrameConnectionTest { + @Rule public TemporaryFolder temporary = new TemporaryFolder(); + + private static final String KEY = "rsa-sha2-256 AAAAB3NzaC1yc2EAAAADAQAB frame-control 'phone'"; + private static final String MAC = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIMac mac"; + + private static final class Result { + final int exit; + final String output; + + Result(int exit, String output) { this.exit = exit; this.output = output; } + } + + private static Result sh(File home, String command) throws Exception { + ProcessBuilder builder = new ProcessBuilder("/bin/sh", "-c", command); + builder.environment().put("HOME", home.getPath()); + builder.redirectError(ProcessBuilder.Redirect.to(new File("/dev/null"))); + Process process = builder.start(); + java.io.ByteArrayOutputStream bytes = new java.io.ByteArrayOutputStream(); + try (java.io.InputStream in = process.getInputStream()) { + byte[] buffer = new byte[4096]; + for (int n; (n = in.read(buffer)) != -1; ) bytes.write(buffer, 0, n); + } + String output = new String(bytes.toByteArray(), StandardCharsets.UTF_8); + assertTrue("Shell timed out", process.waitFor(10, TimeUnit.SECONDS)); + return new Result(process.exitValue(), output); + } + + private String authorize(String existing) throws Exception { + File home = temporary.newFolder(); + File keys = new File(home, ".ssh/authorized_keys"); + if (existing != null) { + assertTrue(keys.getParentFile().mkdirs()); + Files.write(keys.toPath(), existing.getBytes(StandardCharsets.UTF_8)); + } + for (int i = 0; i < 2; i++) assertEquals(0, sh(home, FrameConnection.authorizeCommand(KEY)).exit); + assertEquals("rw-------", PosixFilePermissions.toString(Files.getPosixFilePermissions(keys.toPath()))); + return new String(Files.readAllBytes(keys.toPath()), StandardCharsets.UTF_8); + } + + @Test public void authorizingTerminatesALastLineWithoutNewlineAndIsIdempotent() throws Exception { + assertEquals(KEY + "\n", authorize(null)); + assertEquals(KEY + "\n", authorize("")); + assertEquals(MAC + "\n" + KEY + "\n", authorize(MAC)); + assertEquals(MAC + "\n" + KEY + "\n", authorize(MAC + "\n")); + assertEquals(MAC + "\nother\n" + KEY + "\n", authorize(MAC + "\nother")); + // Already present as an unterminated last line: left exactly as it was. + assertEquals(MAC + "\n" + KEY, authorize(MAC + "\n" + KEY)); + } + + @Test public void accountListingSeparatesAbsentUserdataFromEmptyAccounts() throws Exception { + File home = temporary.newFolder(); + Result absent = sh(home, FrameConnection.ACCOUNTS_COMMAND); + assertEquals(0, absent.exit); + assertEquals("absent\n", absent.output); + assertTrue(FrameConnection.parseAccounts(absent.output).isEmpty()); + File userdata = new File(home, ".local/share/Steam/userdata"); + assertTrue(userdata.mkdirs()); + assertEquals("present\n", sh(home, FrameConnection.ACCOUNTS_COMMAND).output); + for (String directory : Arrays.asList("123", "4567890", "anonymous", "0123456789012", "12a")) + assertTrue(new File(userdata, directory).mkdir()); + Files.write(new File(userdata, "999").toPath(), new byte[]{1}); + Result present = sh(home, FrameConnection.ACCOUNTS_COMMAND); + assertEquals(0, present.exit); + assertEquals(new TreeSet<>(Arrays.asList("123", "4567890")), + FrameConnection.parseAccounts(present.output)); + // An unreadable root is an error, never an empty account list. + File broken = temporary.newFolder(); + File file = new File(broken, ".local/share/Steam/userdata"); + assertTrue(file.getParentFile().mkdirs()); + Files.write(file.toPath(), new byte[]{1}); + assertNotEquals(0, sh(broken, FrameConnection.ACCOUNTS_COMMAND).exit); + } + + @Test public void unexpectedAccountOutputIsRejected() throws Exception { + for (String output : Arrays.asList("", "absent", "present", "present\n123", "absent\n123\n", + "present\nabc\n", "present\n\n", "ready\n", "present\n12 3\n")) { + try { + FrameConnection.parseAccounts(output); + fail("Accepted " + output); + } catch (IOException expected) { + // Fail closed: the poll is retried and no baseline changes. + } + } + assertEquals(Collections.emptySet(), FrameConnection.parseAccounts("present\n")); + } +} diff --git a/android/app/src/test/java/com/saphid/framecontrol/SessionGateTest.java b/android/app/src/test/java/com/saphid/framecontrol/SessionGateTest.java new file mode 100644 index 0000000..c32e38c --- /dev/null +++ b/android/app/src/test/java/com/saphid/framecontrol/SessionGateTest.java @@ -0,0 +1,99 @@ +package com.saphid.framecontrol; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.Assert.*; + +public class SessionGateTest { + @Test public void endedSessionCannotPublishPageOrStop() { + SessionGate gate = new SessionGate(); + List shown = new ArrayList<>(); + long session = gate.begin(); + assertTrue(gate.status(session, "Syncing new screenshots", shown::add)); + assertTrue(gate.page(session, "http://127.0.0.1:1/?key=a")); + assertTrue(gate.end(session)); + assertFalse(gate.running()); + assertNull(gate.page()); + // The worker finishes its download after Stop and reports the closed tunnel. + assertFalse(gate.status(session, "Screenshot pending · Connection reset", shown::add)); + assertFalse(gate.status(session, "Syncing new screenshots", shown::add)); + assertFalse(gate.page(session, "http://127.0.0.1:1/?key=a")); + AtomicBoolean stopped = new AtomicBoolean(); + assertFalse(gate.ifActive(session, () -> stopped.set(true))); + assertFalse(stopped.get()); + assertEquals(Collections.singletonList("Syncing new screenshots"), shown); + assertEquals(SessionGate.DISCONNECTED, gate.state()); + } + + @Test public void lateWorkerCannotChangeANewerSession() { + SessionGate gate = new SessionGate(); + List shown = new ArrayList<>(); + long first = gate.begin(); + gate.end(first); + long second = gate.begin(); + assertTrue(gate.status(second, "Connecting to headset", shown::add)); + assertTrue(gate.page(second, "page-2")); + assertFalse(gate.status(first, "Syncing new screenshots", shown::add)); + assertFalse(gate.page(first, null)); + assertFalse(gate.end(first)); + assertFalse(gate.ifActive(first, () -> fail("Stale session stopped the new one"))); + assertEquals("Connecting to headset", gate.state()); + assertEquals("page-2", gate.page()); + assertTrue(gate.running()); + assertEquals(Collections.singletonList("Connecting to headset"), shown); + long third = gate.begin(); // Even without an end, a newer session supersedes. + assertFalse(gate.status(second, "Syncing new screenshots", null)); + assertTrue(gate.active(third)); + assertNull(gate.page()); + } + + @Test public void endClearsProgressButKeepsFinalError() { + for (String progress : Arrays.asList("Syncing new screenshots", + "Syncing new screenshots · 1 file skipped (empty or over 30 MB)", + "Waiting for headset · Connection refused", "Waiting for a Steam account on the headset", + "Connecting to headset", "Screenshot pending · Download failed")) { + SessionGate gate = new SessionGate(); + long session = gate.begin(); + gate.status(session, progress, null); + gate.end(session); + assertEquals(progress, SessionGate.DISCONNECTED, gate.state()); + } + SessionGate gate = new SessionGate(); + long session = gate.begin(); + gate.status(session, "Pair with your headset first.", null); + gate.end(session); + assertEquals("Pair with your headset first.", gate.state()); + } + + @Test public void endWaitsForAnInFlightNotificationAndNothingPostsAfterIt() throws Exception { + SessionGate gate = new SessionGate(); + long session = gate.begin(); + List events = Collections.synchronizedList(new ArrayList<>()); + CountDownLatch entered = new CountDownLatch(1), release = new CountDownLatch(1); + Thread worker = new Thread(() -> gate.status(session, "Syncing new screenshots", text -> { + events.add("posting"); + entered.countDown(); + try { release.await(5, TimeUnit.SECONDS); } catch (InterruptedException e) { return; } + events.add("posted"); + })); + worker.start(); + assertTrue(entered.await(5, TimeUnit.SECONDS)); + Thread stop = new Thread(() -> { gate.end(session); events.add("ended"); }); + stop.start(); + stop.join(200); + assertTrue("Stop must wait for the notification in flight", stop.isAlive()); + release.countDown(); + worker.join(5000); + stop.join(5000); + assertFalse(gate.status(session, "Syncing new screenshots", text -> events.add("late"))); + assertEquals(Arrays.asList("posting", "posted", "ended"), events); + } +} diff --git a/android/app/src/test/java/com/saphid/framecontrol/SyncEngineTest.java b/android/app/src/test/java/com/saphid/framecontrol/SyncEngineTest.java index 2fc6697..9fdc481 100644 --- a/android/app/src/test/java/com/saphid/framecontrol/SyncEngineTest.java +++ b/android/app/src/test/java/com/saphid/framecontrol/SyncEngineTest.java @@ -13,6 +13,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Set; +import java.util.TreeSet; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; @@ -72,9 +74,32 @@ public class SyncEngineTest { throw new AssertionError("Expected operation to reject invalid or unavailable state"); } + /** Fixture accounts, present on the headset from the first poll onward. */ + private static final Set ACCOUNTS = accounts("123", "124", "123456789012"); + + private static Set accounts(String... ids) { return new TreeSet<>(Arrays.asList(ids)); } + private static void reconcile(SyncEngine engine, Transfer transfer, Gallery gallery, SyncEngine.Shot... shots) throws Exception { - engine.reconcile(Arrays.asList(shots), transfer, gallery); + poll(engine, transfer, gallery, ACCOUNTS, shots); + } + + private static void poll(SyncEngine engine, Transfer transfer, Gallery gallery, Set accounts, + SyncEngine.Shot... shots) throws Exception { + engine.reconcile(new SyncEngine.Listing(accounts, Arrays.asList(shots)), transfer, gallery); + } + + /** Replaces the journal with a directory, so any journal write fails. */ + private static byte[] blockJournal(File root) throws IOException { + File journal = new File(root, "receipts.json"); + byte[] saved = Files.readAllBytes(journal.toPath()); + Files.delete(journal.toPath()); Files.createDirectory(journal.toPath()); + return saved; + } + + private static void unblockJournal(File root, byte[] saved) throws IOException { + File journal = new File(root, "receipts.json"); + Files.delete(journal.toPath()); Files.write(journal.toPath(), saved); } private static void observed(SyncEngine engine, Transfer transfer, Gallery gallery, @@ -388,6 +413,10 @@ public class SyncEngineTest { @Test public void corruptedAndIncompleteJournalsFailClosed() throws Exception { for (String content : new String[]{"", "{", "null", "{}", "{\"gallery\":true}", "{\"version\":2,\"initialized\":true,\"gallery\":true,\"records\":{}}", + "{\"version\":3,\"initialized\":true,\"gallery\":true,\"accounts\":[],\"records\":{}}", + "{\"version\":1,\"initialized\":true,\"gallery\":true,\"accounts\":[],\"records\":{}}", + "{\"version\":2,\"initialized\":true,\"gallery\":true,\"accounts\":[\"x1\"],\"records\":{}}", + "{\"version\":2,\"initialized\":false,\"gallery\":true,\"accounts\":[\"1\"],\"records\":{}}", "{\"version\":1,\"initialized\":true,\"gallery\":true,\"records\":{}} trailing"}) { File root = temporary.newFolder(); File journal = new File(root, "receipts.json"); @@ -442,15 +471,16 @@ public class SyncEngineTest { FIRST + "\n", FIRST + "/extra", "123/456/20261024120000_1.gif"}) { fails(() -> reconcile(engine, transfer, gallery, shot(id))); } - for (long size : new long[]{0, -1, 30L * 1024 * 1024 + 1, Long.MAX_VALUE}) { - fails(() -> reconcile(engine, transfer, gallery, new SyncEngine.Shot(FIRST, size, 1))); - } for (double time : new double[]{Double.NaN, Double.POSITIVE_INFINITY, Double.NEGATIVE_INFINITY}) { fails(() -> reconcile(engine, transfer, gallery, new SyncEngine.Shot(FIRST, 1, time))); } fails(() -> engine.reconcile(null, transfer, gallery)); + fails(() -> engine.reconcile(new SyncEngine.Listing(ACCOUNTS, null), transfer, gallery)); + fails(() -> engine.reconcile(new SyncEngine.Listing(null, Collections.emptyList()), transfer, gallery)); fails(() -> reconcile(engine, transfer, gallery, (SyncEngine.Shot) null)); fails(() -> reconcile(engine, transfer, gallery, A, new SyncEngine.Shot(FIRST, 5, 1))); + // An ineligible duplicate still conflicts with the eligible row of the same id. + fails(() -> reconcile(engine, transfer, gallery, A, new SyncEngine.Shot(FIRST, 0, A.time))); assertTrue(transfer.downloads.isEmpty()); } @@ -786,4 +816,300 @@ public class SyncEngineTest { assertTrue(item(engine, FIRST).file.isFile()); assertTrue(item(engine, SECOND).file.isFile()); } + + // F3: the journal is rewritten only when a receipt changes. + + @Test public void stablePollsDoNotRewriteJournal() throws Exception { + File root = temporary.newFolder(); + SyncEngine engine = new SyncEngine(root, true); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + reconcile(engine, transfer, gallery, A); + byte[] saved = blockJournal(root); + for (int i = 0; i < 3; i++) reconcile(engine, transfer, gallery, A); + reconcile(engine, transfer, gallery); // A missing: a baseline receipt has nothing to reset. + fails(() -> reconcile(engine, transfer, gallery, A, B)); // A new screenshot is persisted. + unblockJournal(root, saved); + reconcile(engine, transfer, gallery, A, B); + reconcile(engine, transfer, gallery, A, B); + assertEquals("saved", item(engine, SECOND).destination); + saved = blockJournal(root); + for (int i = 0; i < 3; i++) reconcile(engine, transfer, gallery, A, B); + assertEquals(Collections.singletonList(SECOND), transfer.downloads); + assertEquals(1, gallery.attempts); + unblockJournal(root, saved); + assertEquals("saved", item(new SyncEngine(root, true), SECOND).destination); + } + + @Test public void deniedExportRetriesEveryPollWithoutJournalWrites() throws Exception { + File root = temporary.newFolder(); + SyncEngine engine = new SyncEngine(root, true); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + observed(engine, transfer, gallery, A); + gallery.deny = true; + reconcile(engine, transfer, gallery, A); + byte[] saved = blockJournal(root); + reconcile(engine, transfer, gallery, A); + reconcile(engine, transfer, gallery); + assertEquals(3, gallery.attempts); + unblockJournal(root, saved); + gallery.deny = false; + reconcile(engine, transfer, gallery, A); + assertEquals("saved", new SyncEngine(root, true).items().get(0).destination); + assertEquals(1, transfer.downloads.size()); + } + + @Test public void failedDownloadRetriesWithoutWritesButMissingPollResetIsPersisted() throws Exception { + File root = temporary.newFolder(); + SyncEngine engine = new SyncEngine(root, false); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + observed(engine, transfer, gallery, A); + transfer.truncated = true; + fails(() -> reconcile(engine, transfer, gallery, A)); + byte[] saved = blockJournal(root); + IOException retry = fails(() -> reconcile(engine, transfer, gallery, A)); + assertTrue(retry.getMessage(), retry.getMessage().contains("Incomplete downloaded image")); + assertEquals(2, transfer.downloads.size()); + // A missing poll changes stability, so it must reach disk; the blocked journal rejects it. + fails(() -> reconcile(engine, transfer, gallery)); + unblockJournal(root, saved); + transfer.truncated = false; + reconcile(engine, transfer, gallery); + SyncEngine relaunched = new SyncEngine(root, false); + reconcile(relaunched, transfer, gallery, A); // One observation after the reset. + assertEquals(2, transfer.downloads.size()); + reconcile(relaunched, transfer, gallery, A); + assertEquals(3, transfer.downloads.size()); + assertTrue(relaunched.items().get(0).file.isFile()); + } + + // F4: an empty or oversized file is skipped per item. + + @Test public void emptyOrOversizedFilesAreSkippedWithoutBlockingBaselineOrOtherScreenshots() throws Exception { + File root = temporary.newFolder(); + SyncEngine engine = new SyncEngine(root, true); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + SyncEngine.Shot empty = new SyncEngine.Shot("123/456/20261024120005_5.jpg", 0, 1); + SyncEngine.Shot huge = new SyncEngine.Shot("124/456/20261024120006_6.png", 30L * 1024 * 1024 + 1, 1); + SyncEngine.Shot negative = new SyncEngine.Shot("123/457/20261024120007_7.jpg", -1, 1); + SyncEngine.Shot maximum = new SyncEngine.Shot("123/458/20261024120008_8.jpg", Long.MAX_VALUE, 1); + reconcile(engine, transfer, gallery, empty, huge, negative, maximum); // Baseline is still set. + assertEquals(4, engine.skipped()); + reconcile(engine, transfer, gallery, empty, huge, A); + reconcile(engine, transfer, gallery, empty, huge, A); + assertEquals(2, engine.skipped()); + assertEquals(Collections.singletonList(FIRST), transfer.downloads); + assertEquals("saved", item(engine, FIRST).destination); + // A capture still being written at baseline time is new once it is complete. + SyncEngine.Shot finished = new SyncEngine.Shot(empty.id, 4, 2); + reconcile(engine, transfer, gallery, finished, huge, A); + // Becoming empty again is an absence: stability restarts. + reconcile(engine, transfer, gallery, empty, huge, A); + reconcile(engine, transfer, gallery, finished, huge, A); + assertEquals(1, transfer.downloads.size()); + reconcile(engine, transfer, gallery, finished, huge, A); + assertEquals(Arrays.asList(FIRST, empty.id), transfer.downloads); + assertEquals(1, engine.skipped()); + assertEquals(2, new SyncEngine(root, true).items().size()); + } + + // F5: the screen reads a library without writing it. + + @Test public void readOnlySnapshotNeverCreatesUpgradesOrRewrites() throws Exception { + File missing = new File(temporary.getRoot(), "missing/library"); + assertTrue(SyncEngine.snapshot(missing).isEmpty()); + assertFalse(missing.getParentFile().exists()); + File root = temporary.newFolder(); + SyncEngine engine = new SyncEngine(root, true); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + observed(engine, transfer, gallery, A); + reconcile(engine, transfer, gallery, A); + engine.setGallery(false); + File journal = new File(root, "receipts.json"); + byte[] before = Files.readAllBytes(journal.toPath()); + java.nio.file.attribute.FileTime modified = Files.getLastModifiedTime(journal.toPath()); + Set entries = new TreeSet<>(Arrays.asList(root.list())); + List items = SyncEngine.snapshot(root); + assertEquals(1, items.size()); + assertEquals("saved", items.get(0).destination); + assertTrue(items.get(0).file.isFile()); + assertArrayEquals(before, Files.readAllBytes(journal.toPath())); + assertEquals(modified, Files.getLastModifiedTime(journal.toPath())); + assertEquals(entries, new TreeSet<>(Arrays.asList(root.list()))); + // A version 1 journal stays version 1 when only viewed. + String legacy = "{\"version\":1,\"initialized\":true,\"gallery\":true,\"records\":{}}"; + Files.write(journal.toPath(), legacy.getBytes(StandardCharsets.UTF_8)); + assertTrue(SyncEngine.snapshot(root).isEmpty()); + assertEquals(legacy, new String(Files.readAllBytes(journal.toPath()), StandardCharsets.UTF_8)); + Files.write(journal.toPath(), "{".getBytes(StandardCharsets.UTF_8)); + fails(() -> SyncEngine.snapshot(root)); + assertEquals("{", new String(Files.readAllBytes(journal.toPath()), StandardCharsets.UTF_8)); + } + + // F1: a closed engine (stopped service) can never write after another engine takes over. + + @Test public void closedEngineCannotWriteAfterAnotherEngineTakesOver() throws Exception { + File root = temporary.newFolder(); + File journal = new File(root, "receipts.json"); + SyncEngine engine = new SyncEngine(root, true); + CountDownLatch entered = new CountDownLatch(1), release = new CountDownLatch(1); + Transfer slow = new Transfer() { + @Override public void download(SyncEngine.Shot shot, File file) throws Exception { + entered.countDown(); await(release); + Files.write(file.toPath(), new byte[]{9, 9, 9, 9}); + } + }; + Transfer fresh = new Transfer(); Gallery gallery = new Gallery(); + observed(engine, fresh, gallery, A); + SyncEngine next; + byte[] saved; + try (Async async = new Async(() -> reconcile(engine, slow, gallery, A))) { + try { + await(entered); + engine.close(); + next = new SyncEngine(root, false); + reconcile(next, fresh, gallery, A); + saved = Files.readAllBytes(journal.toPath()); + } finally { release.countDown(); } + async.finish(); + } + assertArrayEquals(saved, Files.readAllBytes(journal.toPath())); + assertTrue(engine.isClosed()); + assertFalse(engine.setGallery(true)); + try { reconcile(engine, fresh, gallery, A); fail("Closed engine accepted a poll"); } + catch (IllegalStateException expected) { /* Stop is permanent. */ } + assertArrayEquals(saved, Files.readAllBytes(journal.toPath())); + assertArrayEquals(new byte[4], Files.readAllBytes(item(next, FIRST).file.toPath())); + assertEquals("appOnly", item(new SyncEngine(root, false), FIRST).destination); + } + + // F7: each Steam account gets its own baseline the first time it is present. + + @Test public void absentUserdataThenAccountWithHistoryIsBaselinedNotImported() throws Exception { + File root = temporary.newFolder(); + SyncEngine engine = new SyncEngine(root, true); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + poll(engine, transfer, gallery, accounts()); // userdata absent on the first poll + poll(engine, transfer, gallery, accounts("123"), A, B); + poll(new SyncEngine(root, true), transfer, gallery, accounts("123"), A, B); + engine = new SyncEngine(root, true); + poll(engine, transfer, gallery, accounts("123"), A, B); + assertTrue(transfer.downloads.isEmpty()); + assertTrue(engine.items().isEmpty()); + SyncEngine.Shot later = shot("123/456/20261024120002_3.jpg"); + poll(engine, transfer, gallery, accounts("123"), A, B, later); + poll(engine, transfer, gallery, accounts("123"), A, B, later); + assertEquals(Collections.singletonList(later.id), transfer.downloads); + } + + @Test public void establishedAccountWithoutScreenshotsImportsItsFirstCapture() throws Exception { + SyncEngine engine = new SyncEngine(temporary.newFolder(), false); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + poll(engine, transfer, gallery, accounts("123")); + poll(engine, transfer, gallery, accounts("123"), A); + poll(engine, transfer, gallery, accounts("123"), A); + assertEquals(Collections.singletonList(FIRST), transfer.downloads); + } + + @Test public void newAccountIsBaselinedEvenWhenListedOnlyByItsScreenshots() throws Exception { + SyncEngine engine = new SyncEngine(temporary.newFolder(), false); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + SyncEngine.Shot other = shot("124/456/20261024120000_1.jpg"); + SyncEngine.Shot raced = shot("125/456/20261024120000_1.jpg"); + poll(engine, transfer, gallery, accounts("123")); + poll(engine, transfer, gallery, accounts("123", "124"), A, other); + // Account 125 appeared between the account and screenshot queries. + poll(engine, transfer, gallery, accounts("123", "124"), A, other, raced); + poll(engine, transfer, gallery, accounts("123", "124", "125"), A, other, raced); + assertEquals(Collections.singletonList(FIRST), transfer.downloads); + SyncEngine.Shot next = shot("125/456/20261024120009_9.jpg"); + poll(engine, transfer, gallery, accounts("123", "124", "125"), A, other, raced, next); + poll(engine, transfer, gallery, accounts("123", "124", "125"), A, other, raced, next); + assertEquals(Arrays.asList(FIRST, next.id), transfer.downloads); + } + + @Test public void removedAndRestoredAccountIsBaselinedAgainButKeepsItsReceipts() throws Exception { + File root = temporary.newFolder(); + SyncEngine engine = new SyncEngine(root, false); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + poll(engine, transfer, gallery, accounts("123")); + poll(engine, transfer, gallery, accounts("123"), A); + poll(engine, transfer, gallery, accounts()); + engine = new SyncEngine(root, false); + poll(engine, transfer, gallery, accounts("123"), A, B); // B returns as history. + poll(engine, transfer, gallery, accounts("123"), A, B); + assertEquals(Collections.singletonList(FIRST), transfer.downloads); + assertEquals(1, engine.items().size()); + } + + @Test public void invalidAccountListRejectsWholePoll() throws Exception { + SyncEngine engine = new SyncEngine(temporary.newFolder(), false); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + for (String account : new String[]{"", "../123", "12a", "1234567890123", " 123", null}) { + Set bad = new java.util.HashSet<>(Arrays.asList("123", account)); + fails(() -> poll(engine, transfer, gallery, bad, A)); + } + poll(engine, transfer, gallery, accounts("123"), A); // Still the first, baseline poll. + poll(engine, transfer, gallery, accounts("123"), A); + poll(engine, transfer, gallery, accounts("123"), A); + assertTrue(transfer.downloads.isEmpty()); + } + + @Test public void versionOneJournalUpgradesAndOnlyTrustsAccountsItHasSeen() throws Exception { + File root = temporary.newFolder(); + File journal = new File(root, "receipts.json"); + String legacy = "{\"version\":1,\"initialized\":true,\"gallery\":false,\"records\":{\"" + + FIRST + "\":{\"shot\":{\"id\":\"" + FIRST + "\",\"size\":4,\"time\":100.5}," + + "\"state\":\"baseline\",\"observations\":0}}}"; + Files.write(journal.toPath(), legacy.getBytes(StandardCharsets.UTF_8)); + SyncEngine engine = new SyncEngine(root, false); + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + SyncEngine.Shot history = shot("124/456/20261024120000_1.jpg"); + poll(engine, transfer, gallery, accounts("123", "124"), A, B, history); + String upgraded = new String(Files.readAllBytes(journal.toPath()), StandardCharsets.UTF_8); + assertTrue(upgraded, upgraded.contains("\"version\":2")); + assertTrue(upgraded, upgraded.contains("\"accounts\":[\"123\",\"124\"]")); + poll(new SyncEngine(root, false), transfer, gallery, accounts("123", "124"), A, B, history); + assertEquals(Collections.singletonList(SECOND), transfer.downloads); + } + + // R1: a Gallery change before the first poll keeps a version 1 journal valid. + + @Test public void versionOneJournalStaysValidWhenGalleryChangesBeforeFirstPoll() throws Exception { + for (boolean viaConstructor : new boolean[]{true, false}) { + File root = temporary.newFolder(); + File journal = new File(root, "receipts.json"); + String legacy = "{\"version\":1,\"initialized\":true,\"gallery\":true,\"records\":{\"" + + FIRST + "\":{\"shot\":{\"id\":\"" + FIRST + "\",\"size\":4,\"time\":100.5}," + + "\"state\":\"baseline\",\"observations\":0},\"" + + SECOND + "\":{\"shot\":{\"id\":\"" + SECOND + "\",\"size\":4,\"time\":100.5}," + + "\"state\":\"queued\",\"observations\":2,\"destination\":\"pending\"}}}"; + Files.write(journal.toPath(), legacy.getBytes(StandardCharsets.UTF_8)); + // Constructor: the saved preference differs. Setter: the toggle changes while offline. + if (viaConstructor) new SyncEngine(root, false); + else assertTrue(new SyncEngine(root, true).setGallery(false)); + String written = new String(Files.readAllBytes(journal.toPath()), StandardCharsets.UTF_8); + assertTrue(written, written.contains("\"version\":1")); + assertTrue(written, written.contains("\"gallery\":false")); + assertFalse(written, written.contains("accounts")); + // Reopening immediately works for the screen and for a later session. + assertEquals("appOnly", SyncEngine.snapshot(root).get(0).destination); + SyncEngine engine = new SyncEngine(root, false); + assertEquals("appOnly", item(engine, SECOND).destination); + assertTrue(engine.setGallery(true)); + assertTrue(engine.setGallery(false)); + new SyncEngine(root, true); + engine = new SyncEngine(root, false); + // The first poll still upgrades with the version 1 baseline policy. + Transfer transfer = new Transfer(); Gallery gallery = new Gallery(); + SyncEngine.Shot history = shot("124/456/20261024120000_1.jpg"); + poll(engine, transfer, gallery, accounts("123", "124"), A, B, history); + String upgraded = new String(Files.readAllBytes(journal.toPath()), StandardCharsets.UTF_8); + assertTrue(upgraded, upgraded.contains("\"version\":2")); + assertTrue(upgraded, upgraded.contains("\"accounts\":[\"123\",\"124\"]")); + poll(new SyncEngine(root, false), transfer, gallery, accounts("123", "124"), A, B, history); + assertEquals(Collections.singletonList(SECOND), transfer.downloads); + assertEquals(0, gallery.attempts); + assertEquals("appOnly", item(new SyncEngine(root, false), SECOND).destination); + } + } } diff --git a/docs/android-phone.md b/docs/android-phone.md index 8f937f0..eb31230 100644 --- a/docs/android-phone.md +++ b/docs/android-phone.md @@ -20,12 +20,28 @@ implementation, separate from the headset's `frame/android` Lepton tools. items without broad photo-reading permission. The first successful listing establishes a history baseline. Existing screenshots -are not imported automatically. The app waits for two equal size/time listings, -checks the original's byte count and image decode, and saves it atomically. -Identity includes the paired headset, Steam account, game and filename. +are not imported automatically. The baseline is kept per Steam account: the app +also lists the headset's account folders, so an account that is present (even +with no screenshots yet) is established, and its later captures download. If +Steam's userdata folder is missing, or an account folder appears later (a new +sign-in, or one removed and restored), that account's screenshots at that moment +become its baseline instead of being imported. The status reads "Waiting for a +Steam account on the headset" while no account exists. + +The app waits for two equal size/time listings, checks the original's byte count +and image decode, and saves it atomically. Identity includes the paired headset, +Steam account, game and filename. An empty (still being written) or over-30 MB +file is skipped for that poll only and does not hold up other screenshots; the +status shows how many were skipped. A capture still being written when the +baseline is taken downloads once it is complete. Unsafe identifiers, non-finite +times and conflicting duplicate rows still reject the whole listing. The receipt +journal is rewritten only when a receipt changes, not on every poll. Turning Gallery off keeps automatic downloads inside Frame Control. Pending exports become app-only; enabling it again does not export those earlier items. +Turning it off also removes this app's hidden, unfinished Gallery rows left by an +interrupted export, for every retained headset library (best effort; retried +when sync starts with Gallery off). Published Gallery images are never removed. Previously saved Gallery items stay there. Original files remain available from **Open / share original** when disconnected. The app keeps each headset's library separately. @@ -34,6 +50,9 @@ Gallery export uses a deterministic app-owned MediaStore row. A pending row is rewritten after interruption, and a completed row is recognized before inserting another. A failed export leaves the app original available for retry. Preferences and the final Gallery commit are serialized so a disabled setting is respected. +Gallery exports are also serialized across sync sessions, so an export still +finishing after **Stop syncing** cannot remove or overwrite an image that a newer +session saves. **Headset controls** opens the shared Frame Control web interface through a loopback-only SSH tunnel. The server has a fresh session key and listens only on @@ -46,14 +65,17 @@ web bridges; native screenshot originals use the downloads screen. Sync is an explicit, user-started Android `connectedDevice` foreground service. It can continue after leaving the app while its notification is present and reconnects when the headset returns. It is not a Firebase push receiver. -Android battery-saving/Doze, loss of network, force-stop or process termination -can delay or end the session. The app does not request battery-optimization +Android battery-saving/Doze, a locked phone with its screen off, loss of +network, force-stop or process termination can delay or end the session. The app +holds no wake lock; downloads may wait until the phone wakes. The app does not request battery-optimization exemption, use unrelated background modes, or restart itself after force-stop. Restart sync from the app if the session ends. The full cross-platform request still requires actual locked-phone background proof. iOS APNs delivery is a separate unimplemented dependency. An emulator test is not evidence that either physical platform meets that requirement. +Stopping sync ends that session's status and notification; a transfer still +finishing in the background cannot bring them back or change a newer session. Do not publish this as a verified always-on screenshot feature. ## Build and verify @@ -63,7 +85,7 @@ Use JDK 17 and an Android SDK containing platform 36/build-tools 36.0.0: ```sh export JAVA_HOME=/path/to/jdk17 export ANDROID_HOME=/path/to/android-sdk -./android/gradlew -p android assembleDebug testDebugUnitTest lintDebug +./android/gradlew -p android assembleDebug testDebugUnitTest assembleDebugAndroidTest lintDebug ``` The Gradle wrapper pins version 9.4.1 and its distribution checksum. The build diff --git a/docs/evidence/android-phone-review-fixes-2026-10-26.md b/docs/evidence/android-phone-review-fixes-2026-10-26.md new file mode 100644 index 0000000..b71ab2d --- /dev/null +++ b/docs/evidence/android-phone-review-fixes-2026-10-26.md @@ -0,0 +1,97 @@ +# Android phone companion: review fixes (2026-10-26) + +Base: `ae9df354`. This evidence covers fixes for seven findings from an +independent review of the Android companion (issue #108). + +## Status of earlier evidence + +The APK, emulator and physical-device evidence in +`android-phone-screenshot-sync-2026-10-24.md` was captured from `ae9df354`. +It is **historical**. It does not show that the fixes below work on a device. +No emulator, phone or headset was used for the initial implementation round. +Later parent verification is recorded below. + +## What changed + +| Finding | Fix | Covered by | +| --- | --- | --- | +| F1 Stop could leave an ongoing notification or wrong status | `SessionGate` gives each service instance one session token, so an ended session cannot post status, page or notification updates. `SyncEngine.close()` stops all writes before a new session opens the library. `onDestroy` also cancels notification 108. | `SessionGateTest` (4), `closedEngineCannotWriteAfterAnotherEngineTakesOver` | +| F2 Pairing could merge a key into an unterminated last line | `FrameConnection.authorizeCommand` uses the same newline guard as the iPhone app. | `FrameConnectionTest` runs the generated shell against a temporary HOME | +| F3 Every 4-second poll rewrote and fsynced the journal | `reconcile` commits only when a receipt or the account set changes. | 3 tests that turn any journal write into a failure | +| F4 One empty or over-30 MiB file blocked every listing | Out-of-range sizes are skipped for that poll only. Unsafe ids, non-finite times and conflicting duplicates still reject the whole listing. | `emptyOrOversizedFiles…` | +| F5 The screen parsed, and could write, the journal on the UI thread | `SyncEngine.snapshot` is a read-only view. The screen loads it on a background thread, drops results for a headset that is no longer paired, and clears the list when the pairing changes. | `readOnlySnapshotNeverCreatesUpgradesOrRewrites`; existing native `changingHeadsetsRebindsShareToTheSelectedOriginal` compiles | +| F6 An opt-out after an interrupted export left a hidden pending row | `GalleryExporter.discardPending` removes this app's pending rows in `Pictures/Frame Control/` when Gallery is turned off, and again when sync starts with Gallery off. Published rows are kept. | Native `optOutDiscardsInterruptedPendingRowsButKeepsPublishedImages` (compiled, not run) | +| F7 Missing userdata, or a new account, could import its history | Each Steam account gets its own baseline. The app lists the account folders over SSH, so an account with no screenshots yet still counts as established. Version 1 journals upgrade to version 2 and trust only the accounts they already have receipts for. `ui/server.py` is unchanged. | 6 JVM tests plus `accountListingSeparatesAbsentUserdataFromEmptyAccounts` | + +Nonblocking items: the workflow's `push` trigger is now `main` only, and the +docs and app text now say that a locked phone can delay transfers. + +## Verification + +Command (JDK 17, Android SDK platform 36): + +```sh +./android/gradlew -p android clean assembleDebug testDebugUnitTest assembleDebugAndroidTest lintDebug --no-daemon +``` + +- Exit 0. There are 53 JVM tests (34 before), all passing, none skipped. +- Lint reports 0 errors and 12 warnings. All 12 are pre-existing kinds (version hints, i18n, WebView JavaScript). +- Mutation check: restoring the old unconditional commit and the old key append made 4 tests fail. +- `app-debug.apk` SHA-256: `e4a1f498fef6fbc9919ccb4077b8e15b609caea6a2192d3f1bd2b13ea78bcb15` +- `app-debug-androidTest.apk` SHA-256: `f9acb6bc002665cff6a31d0aaf2d528f4b1eaaaa82ead9fb383867e1154f6053` + +## Round 2: fixes for the independent Sol review + +An independent review (GPT-6.1 Sol, high reasoning) of the fixes above found two +product defects and one test timing defect. All three are addressed below. + +| Finding | Fix | Covered by | +| --- | --- | --- | +| R1 A Gallery change before the first poll on a version 1 journal wrote an unreadable journal | A setting-only write now keeps the journal's version, so a version 1 journal stays a valid version 1 journal. Only the first poll upgrades it, and it writes version 2 and the account set together. The baseline policy is unchanged. | JVM `versionOneJournalStaysValidWhenGalleryChangesBeforeFirstPoll` (constructor and toggle paths, immediate reopen and snapshot, then the upgrade). Native `galleryOptOutLatchesPreviouslyPairedLibraries` now includes a retained version 1 library. | +| R2 After Stop, a late export from the old session could delete the Gallery image that the next session had just saved | `GalleryExporter.EXPORTS` is one process-wide lock. Each export holds it from the row lookup through publish or cancellation, across exporter instances and sessions. It is taken before the settings lock and never while holding it. The wait is interruptible and limited to 30 seconds. After that, the item fails for this poll and is retried. | Native `stoppedSessionExportCannotDeleteNextSessionsGalleryImage`. A JVM harness outside the repository runs the real exporter and engine with an in-memory resolver: with the lock, 10 of 10 runs keep the image; with the lock removed, 10 of 10 runs lose it. | +| R3 The headset-switch share test assumed the background library load had finished | The test waits, up to a time limit, until the expected original is shareable. It accepts no button while a library loads. It then checks that only one share button remains and that it shares the new headset's original. | `changingHeadsetsRebindsShareToTheSelectedOriginal` | + +The parent's first native run (API 36, APK `e4a1f498…`) ran 5 tests: 4 passed. +`optOutDiscardsInterruptedPendingRowsButKeepsPublishedImages` reached and +passed its assertions, then its cleanup threw `SecurityException` deleting a +row the product had already removed. The test now drops that row from cleanup +after it proves the row is gone. + +The parent also ran the account-listing command read-only against the real +headset. It exited 0 and listed one account. That is not proof of the phone's +download pipeline. + +Round 2 verification (same command, JDK 17, platform 36): + +- Exit 0. There are 54 JVM tests, all passing: `SyncEngineTest` 47, `SessionGateTest` 4, `FrameConnectionTest` 3. +- Lint reports 0 errors and 12 warnings, the same pre-existing kinds as round 1. +- Mutation check for R1: removing the version-preserving line made only the new R1 test fail, with the invalid version 2 header the review described. +- Two clean builds gave the same hashes. They replace the round-1 APKs: + - `app-debug.apk` SHA-256: `139443c8f3246527ed629731b4f2a099939a2ba7590b628cde415f3f0d7ab996` + - `app-debug-androidTest.apk` SHA-256: `3a84246a7b40436fd5fc521154541603050805270e21e052e26b31a5fa1e0cc2` + +## Parent verification of the round-2 APKs + +The parent installed both round-2 APKs above on the isolated API 36 emulator +and ran the six non-live instrumentation methods explicitly. All six passed, +none skipped; the supervisor exited 0. This includes actual MediaStore +round-trip and retry, retained version 1 library opt-out, asynchronous +headset-switch sharing, hidden pending-row removal, and the FIFO-controlled +Stop/restart export overlap. The chooser was intercepted; no original was +sent to another app. + +The native overlap test exercised real MediaProvider rows: after both old and +new exporters finished, the successor's image remained published with matching +original bytes. This verifies the overlap mechanism, rather than a complete +service Stop/restart against a live headset. + +The parent also checked the 54 passing JVM XML results, the build's recorded +exit 0, whitespace, and both APK hashes. Native output and sanitized metadata +are in `android-phone-review-fixes-2026-10-26/` beside this document. + +## Not verified + +- API 29 runtime behavior and physical Android locked/Doze behavior remain unverified. +- The account-listing command ran against the real headset; the final phone download pipeline and password-pairing UI have not been rerun against it. +- A complete service Stop/restart during a live headset download is not proven by the exporter overlap test. +- iOS APNs delivery, physical locked-phone Photos export, production signing, and release remain open. diff --git a/docs/evidence/android-phone-review-fixes-2026-10-26/api36-instrumentation.txt b/docs/evidence/android-phone-review-fixes-2026-10-26/api36-instrumentation.txt new file mode 100644 index 0000000..9a9c91b --- /dev/null +++ b/docs/evidence/android-phone-review-fixes-2026-10-26/api36-instrumentation.txt @@ -0,0 +1,101 @@ + + + +1 +Performing Streamed Install +Success +Performing Streamed Install +Success +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=1 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream= +com.saphid.framecontrol.ScreenshotDeviceTest: +INSTRUMENTATION_STATUS: test=changingHeadsetsRebindsShareToTheSelectedOriginal +INSTRUMENTATION_STATUS_CODE: 1 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=1 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream=. +INSTRUMENTATION_STATUS: test=changingHeadsetsRebindsShareToTheSelectedOriginal +INSTRUMENTATION_STATUS_CODE: 0 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=2 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream= +INSTRUMENTATION_STATUS: test=stoppedSessionExportCannotDeleteNextSessionsGalleryImage +INSTRUMENTATION_STATUS_CODE: 1 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=2 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream=. +INSTRUMENTATION_STATUS: test=stoppedSessionExportCannotDeleteNextSessionsGalleryImage +INSTRUMENTATION_STATUS_CODE: 0 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=3 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream= +INSTRUMENTATION_STATUS: test=optOutDiscardsInterruptedPendingRowsButKeepsPublishedImages +INSTRUMENTATION_STATUS_CODE: 1 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=3 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream=. +INSTRUMENTATION_STATUS: test=optOutDiscardsInterruptedPendingRowsButKeepsPublishedImages +INSTRUMENTATION_STATUS_CODE: 0 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=4 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream= +INSTRUMENTATION_STATUS: test=galleryOptOutLatchesPreviouslyPairedLibraries +INSTRUMENTATION_STATUS_CODE: 1 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=4 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream=. +INSTRUMENTATION_STATUS: test=galleryOptOutLatchesPreviouslyPairedLibraries +INSTRUMENTATION_STATUS_CODE: 0 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=5 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream= +INSTRUMENTATION_STATUS: test=actualGalleryOriginalAndAppOnlySurviveRelaunch +INSTRUMENTATION_STATUS_CODE: 1 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=5 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream=. +INSTRUMENTATION_STATUS: test=actualGalleryOriginalAndAppOnlySurviveRelaunch +INSTRUMENTATION_STATUS_CODE: 0 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=6 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream= +INSTRUMENTATION_STATUS: test=interruptedMediaStoreRowIsRewrittenWithoutDuplicate +INSTRUMENTATION_STATUS_CODE: 1 +INSTRUMENTATION_STATUS: class=com.saphid.framecontrol.ScreenshotDeviceTest +INSTRUMENTATION_STATUS: current=6 +INSTRUMENTATION_STATUS: id=AndroidJUnitRunner +INSTRUMENTATION_STATUS: numtests=6 +INSTRUMENTATION_STATUS: stream=. +INSTRUMENTATION_STATUS: test=interruptedMediaStoreRowIsRewrittenWithoutDuplicate +INSTRUMENTATION_STATUS_CODE: 0 +INSTRUMENTATION_RESULT: stream= + +Time: 3.171 + +OK (6 tests) + + +INSTRUMENTATION_CODE: -1 diff --git a/docs/evidence/android-phone-review-fixes-2026-10-26/independent-review.md b/docs/evidence/android-phone-review-fixes-2026-10-26/independent-review.md new file mode 100644 index 0000000..891f950 --- /dev/null +++ b/docs/evidence/android-phone-review-fixes-2026-10-26/independent-review.md @@ -0,0 +1,51 @@ +# Independent Android fixes review — 2026-10-26, round 2 + +**Verdict: no actionable findings in the frozen Android hardening delta.** The previous Sol findings R1, R2 and R3 are addressed. This is a source-slice review, not a release-readiness verdict. + +Reviewer: current T3 Codex runtime, `gpt-6.1-sol`, high reasoning. I performed the review myself, without delegation. + +## Exact target and boundaries + +- Worktree: `/Users/saphid/.t3/worktrees/steam-frame/feat-phone-screenshot-autosync-108`. +- Branch: `feat/phone-screenshot-autosync-108`. +- HEAD: `ae9df3544239afc1f2cab17ec10fe07f6fe9e657`, plus the frozen uncommitted delta. +- All **15 paths** in `/tmp/frame108-android-fixes-review-20261026-r2.json` matched their SHA-256 values before and after review. Manifest SHA-256: `e3417c56926d08faecb6e5beacaf081c9e4fc395ef494c4de999a4b2282900ad`. +- I inspected the complete tracked delta, the untracked SessionGate and test files, the evidence files, and surrounding engine, exporter, service, activity and connection logic. I checked callers and lock order as well as the changed lines. +- I read the original Opus review, first Opus fixes report, previous Sol review and reproductions, latest Opus fixes report, and `docs/evidence/android-phone-review-fixes-2026-10-26.md`. +- No `CODING_STANDARDS.md` exists in the worktree, its ancestor directories, or the primary checkout root. I read `CONTRIBUTING.md`; no relevant violated rule was found. + +I made no repository edits, did not read `.frame_pwd` or unrelated dirty primary-checkout files, and used no device, headset or browser. I made no external writes, commits, pushes, PR changes or releases. Review outputs and local test classes are under `/tmp`. + +## Previous Sol findings + +- **R1 — addressed.** `SyncEngine.java:668-673` now preserves both the existing journal version and its account-set representation in `copy()`. Setting-only writes on v1 stay valid v1. Reconcile upgrades to v2 alongside its account set at `:285-304` and `:337-343`. The new JVM regression covers constructor preference reconciliation and an explicit toggle before any poll, immediate snapshot/reopen, repeated toggles, and the subsequent migration/baseline behavior. It passed in my independent run. The retained-v1 instrumentation path is present and passed in the parent's supplied API 36 log. +- **R2 — addressed.** `GalleryExporter.java:74-84` acquires the process-wide `EXPORTS` lock interruptibly with a 30-second timeout, and releases it in `finally`. It covers deterministic row lookup, byte copying, publication and cancellation deletion across exporter instances. The order is EXPORTS → settings monitor → engine monitor. Neither Gallery toggle nor startup cleanup acquires EXPORTS while holding the settings monitor; I found no inverse acquisition path. The old exporter must finish its row work before its successor recovers/inserts a row. I independently recompiled the frozen exporter and engine and ran the existing bounded HandoffRace reproduction with an in-memory resolver: the successor waited, the old cancellation deleted only its pending row, and the successor's published row survived with a saved receipt. The native FIFO test separately checks actual published-row existence and original bytes; its passing result is parent-supplied evidence, not my own device run. +- **R3 — addressed.** `ScreenshotDeviceTest.java:487-517` tolerates a missing button while the background library loads and waits up to 15 seconds for a share URI different from the previous one. The test verifies A and B bytes, then checks exactly one final share button targeting B. It no longer treats main-queue idleness as library completion. The cleanup adjustment removes only the fixture URI already proven deleted; other cleanup errors remain visible. I inspected this code and the parent's passing instrumentation result; I did not independently execute instrumentation. + +## Fresh pass over the earlier fixes + +- **F1:** SessionGate serializes visible status/page/notification effects against session end. Permanent engine close rejects subsequent reconciles and setting writes, and invalidates in-flight journal commits. Late engine assignment and stale-worker finalization are guarded. Gate and takeover tests passed; complete live-service lifecycle behavior remains outside this review. +- **F2:** the generated authorized_keys command terminates an unterminated existing line before appending and remains idempotent. The actual shell fixture cases passed locally. +- **F3:** unchanged listings no longer persist the journal, while retryable downloads/exports continue. Observation resets and account-set changes still commit. The write-blocking regression tests passed. +- **F4:** invalid sizes are excluded per item; unsafe identities, non-finite times and conflicting duplicates still reject a poll. Stability resets and completion after a skipped observation are covered by passing JVM tests. +- **F5:** library snapshots do not create or rewrite storage. Activity parsing/file checks run on its library executor, live-engine selection matches the requested pin, pin changes clear the old list, and results for a no-longer-selected pin are discarded. I found no further actionable sharing regression. +- **F6:** cleanup selects app-owned pending rows in the expected folder, serialized against final publication, and keeps published images. It remains best effort. Provider failure can leave hidden appOnly bytes, particularly on API 29 if Gallery is re-enabled before another cleanup. This documented residual limit is not a newly discovered blocker. +- **F7:** per-account baselines handle absent userdata, established empty accounts, newly appearing accounts, and accounts observed between the SSH and screenshot queries. The migration preserves the intended baseline policy. The shell/parser and account-baseline tests passed. + +The workflow trigger and documentation changes are consistent with this scope. I found no new concrete regression requiring a fix. + +## Actual verification + +All review-generated artifacts are in `/tmp/frame108-android-sol-review-fixes-20261026-r2-evidence/`. + +1. **Independent JVM execution:** compiled current SyncEngine, SessionGate, FrameConnection, PhoneIdentity and all three JVM test classes with JDK 17, cached declared dependencies and SDK 36 into a fresh `/tmp` classes directory. Direct JUnit run: **54 tests passed**, exit 0. No Gradle task or repository build output was changed. Commands, compile log/exit and JUnit log/exit are saved there. +2. **Independent local handoff execution:** compiled the current GalleryExporter against those classes and the existing HandoffRace Android substitutes. Compile and execution both exited 0. Output ended with `exists=true pending=false receipt=saved` and `RESULT: published row survived`. This reuses the inspected existing harness and proves the application-level interleaving; it is not MediaProvider instrumentation. See `race.log` and `race.rc`. +3. **Parent evidence inspected:** final clean build log and exit file record success; existing XML totals are 47 engine + 4 gate + 3 connection tests, all zero failures/errors/skips. The frozen API 36 instrumentation log records **six passing tests, no skips**, and supervisor exit 0. I did not rerun assemble, lint or instrumentation. The parent's lint result is 0 errors and 12 existing warnings. +4. **APK hashes independently recomputed:** app `139443c8f3246527ed629731b4f2a099939a2ba7590b628cde415f3f0d7ab996`; test `3a84246a7b40436fd5fc521154541603050805270e21e052e26b31a5fa1e0cc2`. Both match the supplied round-2 evidence. +5. **Freeze/whitespace:** all 15 source/evidence hashes matched again after checks; branch and HEAD matched; `git diff --check` exited 0. Final hashes and whitespace result are saved in the evidence directory. + +## Verification limits + +Physical locked/Doze behavior, API 29 runtime behavior, complete service Stop/restart during a live headset download, the final phone-to-headset pipeline, iOS APNs/locked Photos behavior, production signing and release are not established here. A stuck old provider write can delay successor exports; the timed lock makes those attempts retryable but does not cancel the stuck I/O. Gallery-toggle I/O on the main thread is an existing disclosed limitation. + +The successful read-only headset account listing supports that command only. Historical device evidence from the committed ae9df354 implementation does not establish the frozen fixes. These limits do not create an actionable defect in this reviewed slice or justify a release claim. diff --git a/docs/evidence/android-phone-review-fixes-2026-10-26/review.json b/docs/evidence/android-phone-review-fixes-2026-10-26/review.json new file mode 100644 index 0000000..72ba36c --- /dev/null +++ b/docs/evidence/android-phone-review-fixes-2026-10-26/review.json @@ -0,0 +1,32 @@ +{ + "taskId": "node:delegated-task:command%3Amcp%3A3ef9db23-13cf-4959-afe1-b003a3f2d950%3Adelegate-task%3Aframe108-android-sol-review-fixes-20261026-r2", + "providerInstanceId": "codex_secondary", + "model": "gpt-6.1-sol", + "reasoningEffort": "high", + "status": "completed", + "findings": "No actionable findings in the frozen Android hardening delta; R1-R3 addressed.", + "frozenFiles": { + ".github/workflows/android.yml": "aaea48149abd1030f18a844d9f245d9a0ad249152e5df838cd53c8863f7f2220", + "android/app/src/androidTest/java/com/saphid/framecontrol/ScreenshotDeviceTest.java": "87f9629858a75a2ba4ced1a0f4ccc792d03a40bd39fed7e25e211cd6c1d15cbf", + "android/app/src/main/java/com/saphid/framecontrol/FrameConnection.java": "edb7b62b4d9a01e8e6802e8758504044c965f6d1b2939536617a3f57832b13a9", + "android/app/src/main/java/com/saphid/framecontrol/GalleryExporter.java": "b0c0c5331ea1184baf780f67329908d481491e7383bfb98c09733ada84582b5d", + "android/app/src/main/java/com/saphid/framecontrol/MainActivity.java": "8069a9c6ac3ab0e5e6bb315055dd2bef2a28b6f1c0084f8355db237857b736c1", + "android/app/src/main/java/com/saphid/framecontrol/SyncEngine.java": "0096ae5002c7ba5d1c7b8128aeee920eb4f29d8d94ad71d885ce86c5be70c854", + "android/app/src/main/java/com/saphid/framecontrol/SyncService.java": "dd852b8ef7f45fc472a8d790ba7e335040355889ef8d082ba8f1e80a1d4fd690", + "android/app/src/test/java/com/saphid/framecontrol/SyncEngineTest.java": "df1fa6ce8873098ea78fca3f5069e31712eae5eedde54824dd83216b6ac67f09", + "docs/android-phone.md": "a274daed1118be64b83313eaad03ddbf080fbbc7dad6e39ce601843bc227c1a3", + "android/app/src/main/java/com/saphid/framecontrol/SessionGate.java": "470a164820ececcbe897f55b3f4c90498c215d9dd1b93554e08d15fcc22a9fa2", + "android/app/src/test/java/com/saphid/framecontrol/FrameConnectionTest.java": "05010052ef11e9d9e019ae4ccb2959a8931a03b2d23528832d4821d184623727", + "android/app/src/test/java/com/saphid/framecontrol/SessionGateTest.java": "5e1e88bee54c82188ce0a1ac8c37d081565fc41c827c67b7d5ef0070b00ab15e", + "docs/evidence/android-phone-review-fixes-2026-10-26.md": "eb2a961abe935541c04e65d43ad8d61036b73afe8f63696891771f5e8bc6b7b1", + "docs/evidence/android-phone-review-fixes-2026-10-26/api36-instrumentation.txt": "d31e729b455ddd8ff3122e5a68bd2221361e27ef3553ba737447598a39d89bd9", + "docs/evidence/android-phone-review-fixes-2026-10-26/verification.json": "2bd1644b0c77c83f3ce85ae1713baba1ea7c0cedce0161bc35e3eff587bce7a7" + }, + "manifestSha256": "e3417c56926d08faecb6e5beacaf081c9e4fc395ef494c4de999a4b2282900ad", + "independentVerification": { + "jvmTests": 54, + "jvmFailures": 0, + "localExportHandoff": "passed" + }, + "productionReleaseReady": false +} diff --git a/docs/evidence/android-phone-review-fixes-2026-10-26/verification.json b/docs/evidence/android-phone-review-fixes-2026-10-26/verification.json new file mode 100644 index 0000000..958a100 --- /dev/null +++ b/docs/evidence/android-phone-review-fixes-2026-10-26/verification.json @@ -0,0 +1,23 @@ +{ + "platform": "Android API 36 emulator", + "nativeTests": 6, + "nativeFailures": 0, + "nativeSkipped": 0, + "supervisorExit": 0, + "jvmTests": 54, + "jvmFailures": 0, + "jvmErrors": 0, + "jvmSkipped": 0, + "buildExit": 0, + "headsetAccountListing": { + "sshExit": 0, + "state": "present", + "accountCount": 1, + "numericEntriesOnly": true + }, + "apkSha256": { + "android/app/build/outputs/apk/debug/app-debug.apk": "139443c8f3246527ed629731b4f2a099939a2ba7590b628cde415f3f0d7ab996", + "android/app/build/outputs/apk/androidTest/debug/app-debug-androidTest.apk": "3a84246a7b40436fd5fc521154541603050805270e21e052e26b31a5fa1e0cc2" + }, + "productionReleased": false +}