Merge remote-tracking branch 'origin/main' into t3/smoke-orphan-cleanup

# Conflicts:
#	CHANGELOG.md

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 ## Unreleased
 
+- Fix smoke cleanup skipping orphan metadata and Steam shortcuts after a test
+  title directory was already removed. Sweep and verify all six exact reserved
+  smoke IDs; preserve similarly named titles. Make `scripts/frame-smoke.sh`
+  work with POSIX sh and Python 3 without zsh, including from another working
+  directory. Local regressions cover both fixes; no new headset run.
+
 - Skip backup saves and settings reached through a symbolic link anywhere below
   the home folder, and recheck paths before packing each file. Local synthetic
   tests cover redirected Proton prefixes and links changed after discovery;

docs/testing.md

@@ -188,6 +188,11 @@ scripts/frame-smoke.sh           # needs `ssh frame` to work without a password
 scripts/frame-smoke.sh --pair    # also pairs a throwaway key: approve it in the headset
 ```
 
+The wrapper uses POSIX `sh` and Python 3 on macOS or Linux; zsh is not required.
+It resolves the checkout path independently of the current working directory.
+`/path/to/checkout/scripts/frame-smoke.sh --help` is safe to run offline and
+does not contact the headset.
+
 It checks `properties.json` and the status, then installs, launches and
 removes three tiny titles built from bytes by `tests/smoke/tiny_programs.py`
 (an ARM64 and an x86-64 static Linux program that sleep for ten seconds, and
@@ -208,6 +213,14 @@ isn't reachable.
 Smoke process cleanup runs before and after title removal and during the
 initial/final leftover sweeps. It targets only the reserved `fc_smoke_arm64`,
 `fc_smoke_x86_64`, `fc_smoke_exe` IDs and their legacy `fc-smoke-` equivalents.
+Each leftover sweep checks all six IDs, even if no title directory or install
+record remains. It removes their exact `-argv.json`, `-env.json`,
+`-settings.json` and `-framecontrol.json` files, syncs Steam's shortcuts, then
+syncs again to verify that none of those IDs is still registered. Missing
+devkit utilities are copied before syncing and removed at the end if they
+were absent before the run. Metadata or shortcuts that remain, or a reported
+sync failure, fail cleanup. Similarly named titles and their metadata are
+outside the deletion boundary.
 On the target, `tests/smoke/stop_processes.py` requires the SSH user's uid and
 an exact title directory in `/proc` cwd/exe or an individual absolute command
 argument; similarly named folders and shell command substrings do not count.
@@ -231,12 +244,16 @@ The `fc_smoke_exe` launch ran under Proton Experimental (ARM64), and its
 remove step stopped five test-owned wrapper/Proton PIDs before removing the
 title; the following process sweep found none. The final cleanup found no
 test titles or processes, the test desktop was closed, and the battery
-remained at 99%. The `scripts/frame-smoke.sh` zsh wrapper could not run
-on lxso2 because zsh is absent; `python3 tests/smoke/frame_smoke.py` ran
-the same entry point instead.
+remained at 99%. That historical run used the Python entry point directly;
+the current POSIX wrapper has local offline coverage on a PATH without zsh.
+The orphan-metadata and shortcut follow-up has not been rerun on a headset.
 The local regression suite runs real Linux processes in temporary fake title
 directories (including deleted folders and a TERM-resistant wrapper); it checks
-exit, escalation and survival of unrelated processes without SSH or a headset:
+exit, escalation and survival of unrelated processes. Local filesystem commands
+with mocked SSH/Steam also cover orphan metadata, shortcut-only leftovers,
+sync failures and exact ID boundaries. Wrapper tests check offline help,
+argument forwarding and exit status from another working directory. None of
+these local tests contacts a headset:
 
 ```sh
 python3 -m unittest discover -s tests -p test_smoke_cleanup.py -v

scripts/frame-smoke.sh

@@ -1,8 +1,9 @@
-#!/usr/bin/env zsh
+#!/bin/sh
 # Mac or Linux: the headset smoke test. Installs, launches and removes tiny
 # test titles on the Frame (the `frame` alias) and records the results with
 # its BUILD_ID under tests/smoke/results/. See docs/testing.md.
 #
 # Usage: scripts/frame-smoke.sh [--pair]   (--pair needs you in the headset to approve)
-set -euo pipefail
-exec python3 "${0:A:h:h}/tests/smoke/frame_smoke.py" "$@"
+set -eu
+smoke_root=$(CDPATH= cd "$(dirname "$0")/.." && pwd -P)
+exec python3 "$smoke_root/tests/smoke/frame_smoke.py" "$@"

tests/smoke/frame_smoke.py

@@ -203,23 +203,23 @@ def cleanup(smoke):
     never removed. Raises if any owned process, file or shortcut stays.
     """
     problems = []
-    try:
-        ids = {t['id'] for t in frame_titles.list_titles() if t['id'] in stop_processes.IDS}
-    except frame_android.FrameError as e:
-        ids = set()
-        problems.append(f'could not list titles: {e}')
-    ids |= smoke.installed
+    # Directory listings and this run's install tracker cannot reveal orphan
+    # metadata or shortcuts. Always sweep the exact, reserved IDs; never let
+    # arbitrary entries in smoke.installed expand the deletion boundary.
+    ids = stop_processes.IDS
     # Scan every reserved ID, even when a prior run removed its folder and
     # shortcut but left Proton running. Stop before removing any more files.
     stopped = stop_titles(stop_processes.IDS)
     gone = set()
     for gid in sorted(ids):
         try:
+            metadata = ' '.join(f'~/devkit-game/{gid}-{k}.json'
+                                for k in ('argv', 'env', 'settings', 'framecontrol'))
             if ssh(f'test -d ~/devkit-game/{gid} && echo yes || true').strip() == 'yes':
                 frame_titles.remove(gid)
             # Also when remove() stopped part-way, or only the json files are left.
-            ssh(f"rm -f {' '.join(f'~/devkit-game/{gid}-{k}.json' for k in ('argv', 'env', 'settings', 'framecontrol'))}")
-            if ssh(f'ls -d ~/devkit-game/{gid} ~/devkit-game/{gid}-*.json 2>/dev/null || true').strip():
+            ssh(f'rm -f {metadata}')
+            if ssh(f'ls -d ~/devkit-game/{gid} {metadata} 2>/dev/null || true').strip():
                 problems.append(f'{gid} is still on the Frame')
             else:
                 gone.add(gid)
@@ -230,6 +230,8 @@ def cleanup(smoke):
         # It logs a failed sync and exits 0, so read its log; a second run lists
         # what Steam still has registered, which must not include ours.
         try:
+            # A shortcut-only leftover (or a first run) may have no utilities.
+            frame_titles.ensure_utils()
             command = frame_titles.devkit_script('delete') + ' 2>&1'
             first = ssh(command, timeout=120)
             second = ssh(command, timeout=120)

tests/test_smoke_cleanup.py

@@ -119,15 +119,6 @@ class Safety(unittest.TestCase):
             with self.assertRaisesRegex(RuntimeError, 'survived cleanup'):
                 stop_processes.stop(['fc_smoke_exe'], grace=0)
 
-    def test_cleanup_scans_even_when_title_already_removed(self):
-        with mock.patch.object(frame_smoke.frame_titles, 'list_titles', return_value=[]), \
-                mock.patch.object(frame_smoke, 'ssh') as ssh, \
-                mock.patch.object(frame_smoke, 'stop_titles', return_value={'stopped_pids': [42]}) as stop:
-            result = frame_smoke.cleanup(frame_smoke.Smoke())
-            self.assertEqual(stop.call_args_list, [mock.call(stop_processes.IDS)] * 2)
-            self.assertEqual(result['process_cleanup'][0]['stopped_pids'], [42])
-            ssh.assert_not_called()
-
     def test_stop_failure_prevents_file_removal(self):
         with mock.patch.object(frame_smoke.frame_titles, 'list_titles', return_value=[{'id': 'fc_smoke_exe'}]), \
                 mock.patch.object(frame_smoke, 'ssh') as ssh, \
@@ -151,3 +142,164 @@ class Safety(unittest.TestCase):
             self.assertEqual(frame_smoke.main(), 1)
             cycle.assert_not_called()
             self.assertEqual(len(list(Path(folder).glob('*-fake.json'))), 1)
+
+
+@unittest.skipUnless(os.name == 'posix', 'local POSIX shell required')
+class OrphanCleanup(unittest.TestCase):
+    """Run the real cleanup shell commands in a throwaway home; mock Steam/SSH."""
+
+    def setUp(self):
+        self.tmp = tempfile.TemporaryDirectory()
+        self.addCleanup(self.tmp.cleanup)
+        self.home = Path(self.tmp.name)
+        self.games = self.home / 'devkit-game'
+        self.games.mkdir()
+        self.shortcuts = set()
+        self.sync_outputs = None
+        self.events = []
+        self.smoke = frame_smoke.Smoke()
+        for target, name, kwargs in (
+                (frame_smoke, 'ssh', {'side_effect': self.local_ssh}),
+                (frame_smoke, 'stop_titles', {'side_effect': self.stop}),
+                (frame_smoke.frame_titles, 'list_titles', {'return_value': []}),
+                (frame_smoke.frame_titles, 'remove', {'side_effect': self.remove}),
+                (frame_smoke.frame_titles, 'ensure_utils', {}),
+                (frame_smoke.frame_titles, 'devkit_script', {'return_value': 'smoke-sync'})):
+            patcher = mock.patch.object(target, name, **kwargs)
+            setattr(self, name + '_mock', patcher.start())
+            self.addCleanup(patcher.stop)
+
+    def stop(self, ids):
+        self.assertEqual(ids, stop_processes.IDS)
+        self.events.append('stop')
+        return {'stopped_pids': [42] if len(self.events) == 1 else []}
+
+    def remove(self, gid):
+        self.assertIn(gid, stop_processes.IDS)
+        self.events.append('remove')
+        shutil.rmtree(self.games / gid)
+
+    def local_ssh(self, command, timeout=60):
+        self.events.append(command)
+        self.assertEqual(self.events[0], 'stop', 'processes must stop before file cleanup')
+        if command == 'smoke-sync 2>&1':
+            if self.sync_outputs is not None:
+                return self.sync_outputs.pop(0)
+            # Valve logs the registered IDs before reconciling with disk.
+            out = '\n'.join(f'Found Devkit Game registered with Steam Client: {gid!r}'
+                            for gid in sorted(self.shortcuts))
+            self.shortcuts = {gid for gid in self.shortcuts if (self.games / gid).is_dir()}
+            return out
+        return subprocess.run(command, shell=True, check=True, capture_output=True,
+                              text=True, timeout=timeout,
+                              env={**os.environ, 'HOME': str(self.home)}).stdout
+
+    def metadata(self, gid):
+        paths = [self.games / f'{gid}-{kind}.json'
+                 for kind in ('argv', 'env', 'settings', 'framecontrol')]
+        for path in paths:
+            path.write_text('{}')
+        return paths
+
+    def test_orphan_metadata_for_every_reserved_id_is_removed_and_shortcuts_rechecked(self):
+        paths = [path for gid in stop_processes.IDS for path in self.metadata(gid)]
+        self.shortcuts = set(stop_processes.IDS)
+        result = frame_smoke.cleanup(self.smoke)
+        self.assertTrue(all(not path.exists() for path in paths))
+        self.assertFalse(self.shortcuts)
+        self.assertEqual(result['removed'], sorted(stop_processes.IDS))
+        self.assertEqual(result['process_cleanup'], [{'stopped_pids': [42]}, {'stopped_pids': []}])
+        self.assertEqual(self.events[-1], 'stop')
+        self.assertEqual(self.events.count('smoke-sync 2>&1'), 2)
+        self.ensure_utils_mock.assert_called_once_with()
+        self.remove_mock.assert_not_called()
+
+    def test_shortcut_only_leftover_is_synced_without_title_or_install_record(self):
+        self.shortcuts = {'fc_smoke_exe'}
+        frame_smoke.cleanup(self.smoke)
+        self.assertFalse(self.shortcuts)
+        self.assertEqual(self.events.count('smoke-sync 2>&1'), 2)
+
+    def test_unrelated_titles_and_metadata_survive_even_if_in_install_tracker(self):
+        others = ('portal2', 'fc_smoke_custom', 'fc_smoke_exe-other', 'fc_smoke_exe_other')
+        paths = []
+        for gid in ('fc_smoke_exe', *others):
+            (self.games / gid).mkdir()
+            paths.extend(self.metadata(gid) if gid in others else [])
+        self.shortcuts = {'fc_smoke_exe', *others}
+        self.smoke.installed = {'fc_smoke_exe', *others, 'fc_smoke_exe/../portal2'}
+        frame_smoke.cleanup(self.smoke)
+        self.assertFalse((self.games / 'fc_smoke_exe').exists())
+        self.assertTrue(all((self.games / gid).is_dir() for gid in others))
+        self.assertTrue(all(path.exists() for path in paths))
+        self.assertEqual(self.shortcuts, set(others))
+        self.assertNotIn('fc_smoke_exe', self.smoke.installed)
+        self.remove_mock.assert_called_once_with('fc_smoke_exe')
+
+    def test_still_registered_current_or_legacy_shortcut_fails_cleanup(self):
+        for gid in sorted(stop_processes.IDS):
+            with self.subTest(gid=gid):
+                self.sync_outputs = ['', f'Found Devkit Game registered with Steam Client: {gid!r}']
+                self.smoke.installed = {gid}
+                with self.assertRaisesRegex(AssertionError, f'{gid} is still registered with Steam'):
+                    frame_smoke.cleanup(self.smoke)
+                self.assertIn(gid, self.smoke.installed)
+
+    def test_sync_failure_in_either_pass_fails_cleanup(self):
+        for failed_pass in (0, 1):
+            with self.subTest(failed_pass=failed_pass):
+                self.sync_outputs = ['', '']
+                self.sync_outputs[failed_pass] = 'Steam client sync of devkit games failed: offline'
+                self.smoke.installed = {'fc_smoke_exe'}
+                with self.assertRaisesRegex(AssertionError, 'sync of devkit games failed'):
+                    frame_smoke.cleanup(self.smoke)
+                self.assertIn('fc_smoke_exe', self.smoke.installed)
+
+    def test_metadata_remaining_after_delete_fails_cleanup(self):
+        self.metadata('fc_smoke_exe')
+        real_ssh = self.local_ssh
+        with mock.patch.object(frame_smoke, 'ssh', side_effect=lambda command, **kwargs:
+                               '' if command.startswith('rm -f ') else real_ssh(command, **kwargs)):
+            with self.assertRaisesRegex(AssertionError, 'fc_smoke_exe is still on the Frame'):
+                frame_smoke.cleanup(self.smoke)
+
+    def test_missing_utils_cannot_report_success_without_shortcut_verification(self):
+        self.ensure_utils_mock.side_effect = frame_smoke.frame_android.FrameError('copy failed')
+        with self.assertRaisesRegex(AssertionError, "syncing Steam's shortcuts: copy failed"):
+            frame_smoke.cleanup(self.smoke)
+        self.assertEqual(self.events[-1], 'stop')
+
+
+@unittest.skipUnless(os.name == 'posix', 'POSIX wrapper')
+class PortableWrapper(unittest.TestCase):
+    def setUp(self):
+        self.tmp = tempfile.TemporaryDirectory(prefix='smoke wrapper ')
+        self.addCleanup(self.tmp.cleanup)
+        # macOS aliases /var to /private/var; relative commands need the physical cwd.
+        self.folder = Path(self.tmp.name).resolve()
+        self.bin = self.folder / 'bin'
+        self.bin.mkdir()
+        # No zsh or SSH on PATH: --help must exit before any headset access.
+        (self.bin / 'dirname').symlink_to(shutil.which('dirname'))
+        self.env = {**os.environ, 'PATH': str(self.bin), 'CDPATH': str(self.folder)}
+        self.wrapper = SMOKE.parent.parent / 'scripts' / 'frame-smoke.sh'
+
+    def test_help_runs_directly_and_with_posix_sh_from_another_cwd_without_zsh(self):
+        (self.bin / 'python3').symlink_to(sys.executable)
+        for launcher in ([], ['/bin/sh']):
+            with self.subTest(launcher=launcher):
+                result = subprocess.run([*launcher, str(self.wrapper), '--help'], cwd=self.folder,
+                                        env=self.env, capture_output=True, text=True, timeout=10)
+                self.assertEqual(result.returncode, 0, result.stderr)
+                self.assertIn('--pair', result.stdout)
+                self.assertIn('Headset smoke test', result.stdout)
+
+    def test_relative_invocation_preserves_arguments_and_python_exit_status(self):
+        python = self.bin / 'python3'
+        python.write_text('#!/bin/sh\nprintf "<%s>\\n" "$@"\nexit 7\n')
+        python.chmod(0o755)
+        result = subprocess.run([os.path.relpath(self.wrapper, self.folder), '--pair', 'two words', ''],
+                                cwd=self.folder, env=self.env, capture_output=True, text=True, timeout=10)
+        self.assertEqual(result.returncode, 7, result.stderr)
+        self.assertEqual(result.stdout.splitlines(),
+                         [f'<{SMOKE / "frame_smoke.py"}>', '<--pair>', '<two words>', '<>'])