From 7638c06248548c2ebe95e5fe3c7462f086542de4 Mon Sep 17 00:00:00 2001 From: rigidlab Date: Tue, 25 Aug 2026 21:53:19 -0700 Subject: [PATCH 1/3] docs: record how config is shared between qtxterm instances SPEC.md gains Phase 4s.1, covering why the config files are polled once a minute rather than watched, and stating the two limits this does not remove: the whole-file save still resolves last-writer-wins inside the same minute, and each instance still fires every job into its own tab. USAGE.md says the same in user terms, in the Cron section. Code assisted by Opus 5. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M8UgLA68nRCYg3qFTyW8ms --- SPEC.md | 55 ++++++++++++++++++++++++++++++++++++- src/qtxterm/assets/USAGE.md | 7 ++++- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/SPEC.md b/SPEC.md index 3c2582f..09fbb07 100644 --- a/SPEC.md +++ b/SPEC.md @@ -815,6 +815,58 @@ Four decisions, each taken over a plausible alternative: the preset menus rather than growing its own copy of the ownership handling those exist for. +### Phase 4s.1 - Config shared between instances ✅ done +Two qtxterm windows are two processes over one `cron.json` and one +`presets.json`. Each loaded its file once at launch, so a job added in one +was invisible to the other until restart - and the next save from the stale +window wrote its old list over the new one, losing the job silently. + +- [x] **Polled once a minute, not watched.** The scheduler's tick already + runs every second, so checking on the minute costs a `stat()` and *no + extra wakeup*. A `QFileSystemWatcher` costs a worker thread, wakes on + every change in the config directory, and stops watching a file that + is *replaced* rather than modified - which is exactly what the atomic + save below does. A minute is also cron's own resolution: there is + nothing to do with a job noticed sooner than it could first fire. +- [x] **Stamped, so an unchanged file is never re-parsed.** `(mtime_ns, + size)`, compared before reading. Reloading unconditionally would emit + `changed` every minute and rebuild every menu, including one the user + has open. +- [x] **Presets are polled too, not just jobs.** A job added elsewhere + normally arrives with the Macro it names, and a job whose Macro has + not been re-read fails every single firing with "no Macro named ... + any more". +- [x] **Atomic save**, via a sibling temp file and `os.replace`. The reader + is another process polling this exact path; a plain write is visible in + its truncated middle. +- [x] **Reloads are suspended while an editor is open.** Both editors address + entries by index, and an `exec()` dialog still runs the event loop - a + poll landing mid-edit would shift the list under the open form and save + the user's changes onto whichever entry inherited that index. Counted, + not a flag, since two editors can be open over one store. +- [x] **A bad read changes nothing and is retried.** Invalid JSON - a + hand-edit in progress, a half-synced cloud drive, an older qtxterm + without the atomic save - leaves the in-memory list alone and does not + advance the stamp. Raising out of a timer would take down the window + over a file that is very likely fine a second later. A *deleted* file + is treated the same way: the presets do not evaporate mid-session. + +Two things this deliberately does not fix: + +- **The clobber window is narrowed, not closed.** Saves still write the whole + file, so an edit made in two windows inside the same minute still resolves + last-writer-wins. Closing it properly means merging per entry, which needs + stable ids the files do not have. +- **Both instances still fire the same job**, each into its own tab, since + each runs its own scheduler. Cron is per-instance by design. If that ever + needs to change, the fix is a single-instance lock at launch, not a + fourth thing for the poll to do. + +Implementation note: `config_store.py` holds this once and both stores +inherit it - `PresetStore` and `CronStore` were already the same problem +twice (a list of dataclasses, one JSON file, editors addressing entries by +index), and the file handling is now the part they literally share. + Implementation notes: - `cron.py` is schedules and storage and knows nothing about terminals; @@ -822,7 +874,8 @@ Implementation notes: what let the whole expression layer be tested without a Qt widget. - The scheduler ticks every second and acts only when the *minute* changes. A 60s timer drifts against the wall clock and skips a minute whenever the - machine sleeps. + machine sleeps. On each minute change it re-reads the config first, then + fires - see Phase 4s.1. - `next_run()` walks minute by minute but skips whole days that cannot match, and gives up after four years - "0 0 31 2 *" parses fine and can never happen, and the alternative to a bound is a UI that hangs. diff --git a/src/qtxterm/assets/USAGE.md b/src/qtxterm/assets/USAGE.md index 20d0952..b145cd3 100644 --- a/src/qtxterm/assets/USAGE.md +++ b/src/qtxterm/assets/USAGE.md @@ -364,7 +364,7 @@ that menu, the same way Macros do - useful once you have a job per feed per session. Ungrouped jobs stay at the top level. **Run Now** in the editor runs a job immediately, which beats waiting until 2am to find out whether it works. -Two things to know: +Three things to know: - **Jobs only run while qtxterm is open**, and nothing missed while it was closed is caught up on. Launching after a weekend does not fire a burst of @@ -372,6 +372,11 @@ Two things to know: machine, use the system's own scheduler. - **A job names its Macro.** Rename or delete that Macro and the job says so in the status bar rather than running something else. +- **Two qtxterm windows share one set of jobs.** Add or edit a job in one and + the other picks it up within a minute, no restart needed. Note that each + window runs its own schedule, so a job fires once per open window, each + into its own tab. If that isn't what you want, keep one window open, or + turn the job off in the others. ## Ordering From 310fc01f8803d947b0b073ad624d0238c67b568e Mon Sep 17 00:00:00 2001 From: rigidlab Date: Tue, 25 Aug 2026 21:53:32 -0700 Subject: [PATCH 2/3] feat: poll cron and preset config for edits from another instance Two qtxterm windows are two processes over one cron.json and one presets.json. Each loaded its file once at launch, so a job added in one was invisible to the other until restart, and the next save from the stale window wrote its old list over the new one. New ConfigStore holds the file handling both stores already shared: a stamped reload that stats before it parses, an atomic save so a polling reader never sees a truncated file, and a counted suspend the editors hold while open - they address entries by index, and an exec() dialog still runs the event loop. CronScheduler re-reads on each minute change, before firing. Polled there rather than watched because that tick already runs every second, so it costs a stat and no extra wakeup, and because QFileSystemWatcher stops watching a file that is replaced - which is what the atomic save does. Presets are refreshed as well as jobs, since a job whose Macro has not been re-read fails every firing. Code assisted by Opus 5. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M8UgLA68nRCYg3qFTyW8ms --- src/qtxterm/config_store.py | 150 ++++++++++++++++++++++++++++++++++ src/qtxterm/cron.py | 27 +++--- src/qtxterm/cron_editor.py | 9 ++ src/qtxterm/cron_scheduler.py | 22 +++++ src/qtxterm/preset_editor.py | 4 + src/qtxterm/presets.py | 36 ++++---- 6 files changed, 210 insertions(+), 38 deletions(-) create mode 100644 src/qtxterm/config_store.py diff --git a/src/qtxterm/config_store.py b/src/qtxterm/config_store.py new file mode 100644 index 0000000..20a06d8 --- /dev/null +++ b/src/qtxterm/config_store.py @@ -0,0 +1,150 @@ +"""Shared file handling for the JSON config stores. + +`PresetStore` and `CronStore` are the same problem twice: a list of +dataclasses in one JSON file under the user's config dir, edited by dialogs +that address entries by index. This holds the parts that are identical - the +atomic write, and picking up a file a *second qtxterm instance* has written. + +On the second instance: each process loads its config once at launch and +keeps it in memory, so without this a job added in one window is invisible to +the other, and the next save from the stale window writes its old list over +the new one. Polling closes most of that gap - see `reload_if_changed`. +""" + +from __future__ import annotations + +import json +import os +from contextlib import contextmanager +from pathlib import Path + +from PySide6.QtCore import QObject, Signal + +# What we compare to decide the file changed under us. mtime alone is too +# coarse on filesystems with a 1-2s timestamp granularity (FAT, some network +# shares), where two writes in the same second look identical; the size +# catches the common case of those two writes differing in length. +_Stamp = tuple[int, int] | None + + +class ConfigStore(QObject): + """A JSON list on disk, reloaded when another process rewrites it. + + Subclasses own the shape of the payload (`_apply_payload` / + `_build_payload`) and what an absent file means (`_apply_missing`); this + class owns the bytes. + """ + + changed = Signal() + + def __init__(self, path: Path) -> None: + super().__init__() + self.path = path + self._stamp: _Stamp = None + self._suspended = 0 + + # -- subclass hooks ---------------------------------------------------- + + def _apply_payload(self, raw: list) -> None: + raise NotImplementedError + + def _build_payload(self) -> list: + raise NotImplementedError + + def _apply_missing(self) -> None: + """Called by load() when the file does not exist yet.""" + raise NotImplementedError + + # -- file handling ----------------------------------------------------- + + def load(self) -> None: + if not self.path.exists(): + self._stamp = None + self._apply_missing() + return + self._apply_payload(json.loads(self.path.read_text(encoding="utf-8"))) + self._stamp = self._current_stamp() + + def save(self) -> None: + """Write the whole file, atomically, and announce it. + + Atomically because another instance may be polling this exact file: + a plain write is visible in its truncated middle, and the reader gets + a JSONDecodeError instead of a config. `os.replace` means a reader + sees either the old file or the new one. + """ + self.path.parent.mkdir(parents=True, exist_ok=True) + text = json.dumps(self._build_payload(), indent=2) + temp = self.path.with_name(f"{self.path.name}.{os.getpid()}.tmp") + try: + temp.write_text(text, encoding="utf-8") + os.replace(temp, self.path) + except OSError: + temp.unlink(missing_ok=True) + raise + self._stamp = self._current_stamp() + self.changed.emit() + + def reload_if_changed(self) -> bool: + """Re-read if another process has rewritten the file. Did we reload? + + Cheap enough to call on a timer: a stat, and a parse only when the + stamp moved. Callers get `changed` exactly as if the edit had been + made here, so every menu and sidebar already knows what to do with it. + + Never raises. A file that is unreadable *now* - mid-write by an older + qtxterm without the atomic save, half-synced by a cloud drive, or + hand-edited into invalid JSON - leaves the in-memory list alone and + is retried on the next poll. Throwing from a timer would take out the + window over a file that is very likely fine a second later. + """ + if self._suspended: + return False + stamp = self._current_stamp() + if stamp == self._stamp: + return False + if stamp is None: + # Deleted out from under us. Keeping what we have is the kinder + # reading: the file coming back is a reload, and until then the + # user's presets do not evaporate mid-session. + self._stamp = None + return False + try: + raw = json.loads(self.path.read_text(encoding="utf-8")) + except (OSError, ValueError): + return False + # Stamped only on success, so a failed read is retried rather than + # remembered as done. + self._apply_payload(raw) + self._stamp = stamp + self.changed.emit() + return True + + def suspend_reload(self) -> None: + """Hold off reloads while an editor is open over this store. + + The editors address entries by index (`update(index, ...)`), and a + modal dialog still runs the event loop, so a poll landing mid-edit + would shift the list under the open form and save the user's changes + onto whichever entry inherited that index. Counted rather than a + flag, because more than one editor can be open over one store. + """ + self._suspended += 1 + + def resume_reload(self) -> None: + self._suspended = max(0, self._suspended - 1) + + @contextmanager + def reload_suspended(self): + self.suspend_reload() + try: + yield + finally: + self.resume_reload() + + def _current_stamp(self) -> _Stamp: + try: + info = self.path.stat() + except OSError: + return None + return (info.st_mtime_ns, info.st_size) diff --git a/src/qtxterm/cron.py b/src/qtxterm/cron.py index 413880a..bb492c0 100644 --- a/src/qtxterm/cron.py +++ b/src/qtxterm/cron.py @@ -16,12 +16,12 @@ from __future__ import annotations import dataclasses -import json from datetime import datetime, timedelta from pathlib import Path import platformdirs -from PySide6.QtCore import QObject, Signal + +from qtxterm.config_store import ConfigStore # (name, low, high) per field, in the order they are written. _FIELDS = [ @@ -222,22 +222,18 @@ def default_cron_path() -> Path: return Path(platformdirs.user_config_dir("qtxterm", appauthor=False)) / "cron.json" -class CronStore(QObject): +class CronStore(ConfigStore): """Loads/saves cron jobs as JSON, the same shape PresetStore uses.""" - changed = Signal() - def __init__(self, path: Path | None = None) -> None: - super().__init__() - self.path = path or default_cron_path() + super().__init__(path or default_cron_path()) self.jobs: list[CronJob] = [] self.load() - def load(self) -> None: - if not self.path.exists(): - self.jobs = [] - return - raw = json.loads(self.path.read_text(encoding="utf-8")) + def _apply_missing(self) -> None: + self.jobs = [] + + def _apply_payload(self, raw: list) -> None: # Unknown keys are dropped rather than raising: a file written by a # later version should cost you a field, not the whole app. fields = {f.name for f in dataclasses.fields(CronJob)} @@ -245,11 +241,8 @@ def load(self) -> None: CronJob(**{k: v for k, v in item.items() if k in fields}) for item in raw ] - def save(self) -> None: - self.path.parent.mkdir(parents=True, exist_ok=True) - payload = [dataclasses.asdict(job) for job in self.jobs] - self.path.write_text(json.dumps(payload, indent=2), encoding="utf-8") - self.changed.emit() + def _build_payload(self) -> list: + return [dataclasses.asdict(job) for job in self.jobs] def add(self, job: CronJob) -> None: self.jobs.append(job) diff --git a/src/qtxterm/cron_editor.py b/src/qtxterm/cron_editor.py index 4b17604..a246822 100644 --- a/src/qtxterm/cron_editor.py +++ b/src/qtxterm/cron_editor.py @@ -64,6 +64,11 @@ def __init__( self._preset_store = preset_store self._scheduler = scheduler self._current_index: int | None = None + # This form addresses jobs by index, so the lists behind it must not + # be swapped out by a poll while it is open - see ConfigStore. + self._store.suspend_reload() + self._preset_store.suspend_reload() + self.finished.connect(self._resume_reloads) layout = QHBoxLayout(self) @@ -129,6 +134,10 @@ def __init__( if create_new: self._new_job() + def _resume_reloads(self) -> None: + self._store.resume_reload() + self._preset_store.resume_reload() + def _runnable_presets(self): """Macros only. diff --git a/src/qtxterm/cron_scheduler.py b/src/qtxterm/cron_scheduler.py index 5c21103..4405725 100644 --- a/src/qtxterm/cron_scheduler.py +++ b/src/qtxterm/cron_scheduler.py @@ -27,6 +27,9 @@ class CronScheduler(QObject): the app starts never fires, and nothing missed while it was closed is replayed. Otherwise launching after a weekend would open a burst of terminals before you had touched anything. + + Config is re-read on the minute, so a job added in another instance is + picked up here without a restart - see `refresh_config`. """ job_fired = Signal(str) @@ -73,8 +76,27 @@ def _on_tick(self) -> None: if minute == self._last_minute: return self._last_minute = minute + self.refresh_config() self.run_due_jobs(minute) + def refresh_config(self) -> None: + """Pick up jobs and Macros written by another qtxterm instance. + + Polled here rather than watched, for two reasons. This tick already + happens every second, so checking on the minute costs a stat and no + extra wakeup at all - where a QFileSystemWatcher costs a worker + thread, and silently stops watching when a file is *replaced*, which + is exactly what the atomic save in `ConfigStore` does. And a minute + is the resolution cron works at anyway; there is nothing to be done + with a job noticed sooner than the minute it could first fire. + + Presets as well as jobs, because a job added elsewhere usually + arrives with the Macro it runs, and a job whose Macro we have not + re-read fails every firing with "no Macro named ... any more". + """ + self._preset_store.reload_if_changed() + self._cron_store.reload_if_changed() + def run_due_jobs(self, minute: datetime) -> list[str]: """Fire every enabled job whose schedule matches `minute`.""" fired: list[str] = [] diff --git a/src/qtxterm/preset_editor.py b/src/qtxterm/preset_editor.py index 2d90939..0786966 100644 --- a/src/qtxterm/preset_editor.py +++ b/src/qtxterm/preset_editor.py @@ -121,6 +121,10 @@ def __init__( self.resize(620, 420) self._store = store self._current_index: int | None = None + # Addresses presets by index, so a poll must not reorder the list + # under the open form - see ConfigStore. + self._store.suspend_reload() + self.finished.connect(lambda _: self._store.resume_reload()) self._is_command = category == CATEGORY_COMMANDS self._is_macro = category == CATEGORY_MACROS self._is_selection = category == CATEGORY_SELECTION diff --git a/src/qtxterm/presets.py b/src/qtxterm/presets.py index 8b490a9..124e0a6 100644 --- a/src/qtxterm/presets.py +++ b/src/qtxterm/presets.py @@ -1,11 +1,11 @@ from __future__ import annotations import dataclasses -import json from pathlib import Path import platformdirs -from PySide6.QtCore import QObject, Signal + +from qtxterm.config_store import ConfigStore INPUT_NONE = "none" INPUT_SELECTION = "selection" @@ -173,35 +173,29 @@ def default_presets_path() -> Path: ) -class PresetStore(QObject): +class PresetStore(ConfigStore): """Loads/saves the preset list as JSON, seeding defaults on first run. Emits `changed` after every save() so any number of UI surfaces (sidebar, Macros menu) can stay in sync regardless of which one - triggered the edit, without reaching into each other. + triggered the edit, without reaching into each other - and, since + `ConfigStore` polls, regardless of which *instance* made the edit. """ - changed = Signal() - def __init__(self, path: Path | None = None) -> None: - super().__init__() - self.path = path or default_presets_path() + super().__init__(path or default_presets_path()) self.presets: list[Preset] = [] self.load() - def load(self) -> None: - if self.path.exists(): - raw = json.loads(self.path.read_text(encoding="utf-8")) - self.presets = [Preset(**item) for item in raw] - else: - self.presets = default_presets() - self.save() - - def save(self) -> None: - self.path.parent.mkdir(parents=True, exist_ok=True) - payload = [dataclasses.asdict(p) for p in self.presets] - self.path.write_text(json.dumps(payload, indent=2), encoding="utf-8") - self.changed.emit() + def _apply_missing(self) -> None: + self.presets = default_presets() + self.save() + + def _apply_payload(self, raw: list) -> None: + self.presets = [Preset(**item) for item in raw] + + def _build_payload(self) -> list: + return [dataclasses.asdict(p) for p in self.presets] def sidebar_presets(self) -> list[Preset]: """Command-category presets opted into the sidebar. From ce6006b88a56b348589361295bc592bfe44989fa Mon Sep 17 00:00:00 2001 From: rigidlab Date: Tue, 25 Aug 2026 21:53:33 -0700 Subject: [PATCH 3/3] test: cover cross-instance config reload and editor suspension The "other instance" is a second store over the same path, which is what two windows are. Covers the pickup itself, that an unchanged file is never reparsed, that half-written JSON and a deleted file change nothing and are retried, that an open editor holds reloads off, and that a reload leaves a running job pointed at its own tab. Code assisted by Opus 5. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01M8UgLA68nRCYg3qFTyW8ms --- tests/test_config_store.py | 170 +++++++++++++++++++++++++++++++++++ tests/test_cron_editor.py | 42 +++++++++ tests/test_cron_scheduler.py | 82 +++++++++++++++++ tests/test_preset_editor.py | 24 +++++ 4 files changed, 318 insertions(+) create mode 100644 tests/test_config_store.py diff --git a/tests/test_config_store.py b/tests/test_config_store.py new file mode 100644 index 0000000..dcf8c3e --- /dev/null +++ b/tests/test_config_store.py @@ -0,0 +1,170 @@ +"""ConfigStore: picking up a file a second qtxterm instance has written. + +The "other instance" in these tests is a second store object over the same +path, which is exactly what two running windows are. +""" + +from __future__ import annotations + +import json + +from qtxterm.cron import CronJob, CronStore +from qtxterm.presets import Preset, PresetStore + + +def job(name: str, expression: str = "* * * * *") -> CronJob: + return CronJob(name=name, expression=expression, preset_name="Backup") + + +def test_a_job_added_by_another_instance_is_picked_up(tmp_path) -> None: + path = tmp_path / "cron.json" + here, elsewhere = CronStore(path=path), CronStore(path=path) + + elsewhere.add(job("Nightly")) + + assert here.reload_if_changed() is True + assert [j.name for j in here.jobs] == ["Nightly"] + + +def test_reload_announces_the_change_like_a_local_edit(tmp_path) -> None: + path = tmp_path / "cron.json" + here, elsewhere = CronStore(path=path), CronStore(path=path) + seen = [] + here.changed.connect(lambda: seen.append(len(here.jobs))) + + elsewhere.add(job("Nightly")) + here.reload_if_changed() + + assert seen == [1] + + +def test_an_unchanged_file_is_not_reparsed(tmp_path) -> None: + path = tmp_path / "cron.json" + store = CronStore(path=path) + store.add(job("Nightly")) + seen = [] + store.changed.connect(seen.append) + + # The point of the stamp: polling every minute forever must not rebuild + # every menu every minute. + assert store.reload_if_changed() is False + assert store.reload_if_changed() is False + assert seen == [] + + +def test_our_own_save_does_not_look_like_someone_elses(tmp_path) -> None: + store = CronStore(path=tmp_path / "cron.json") + store.add(job("Nightly")) + + assert store.reload_if_changed() is False + + +def test_reload_is_held_off_while_an_editor_is_open(tmp_path) -> None: + path = tmp_path / "cron.json" + here, elsewhere = CronStore(path=path), CronStore(path=path) + elsewhere.add(job("Nightly")) + + with here.reload_suspended(): + assert here.reload_if_changed() is False + assert here.jobs == [] + + assert here.reload_if_changed() is True + assert [j.name for j in here.jobs] == ["Nightly"] + + +def test_suspension_nests(tmp_path) -> None: + path = tmp_path / "cron.json" + here, elsewhere = CronStore(path=path), CronStore(path=path) + elsewhere.add(job("Nightly")) + + with here.reload_suspended(): + with here.reload_suspended(): + pass + # Still held by the outer editor. + assert here.reload_if_changed() is False + + assert here.reload_if_changed() is True + + +def test_half_written_json_leaves_the_jobs_alone_and_is_retried(tmp_path) -> None: + path = tmp_path / "cron.json" + store = CronStore(path=path) + store.add(job("Nightly")) + + path.write_text('[{"name": "Half', encoding="utf-8") + assert store.reload_if_changed() is False + assert [j.name for j in store.jobs] == ["Nightly"] + + # The stamp was not advanced, so the next poll still sees a change. + path.write_text( + json.dumps([{"name": "Whole", "expression": "* * * * *", "preset_name": "B"}]), + encoding="utf-8", + ) + assert store.reload_if_changed() is True + assert [j.name for j in store.jobs] == ["Whole"] + + +def test_a_deleted_file_does_not_wipe_the_jobs(tmp_path) -> None: + path = tmp_path / "cron.json" + store = CronStore(path=path) + store.add(job("Nightly")) + + path.unlink() + + assert store.reload_if_changed() is False + assert [j.name for j in store.jobs] == ["Nightly"] + + +def test_unknown_fields_still_survive_a_reload(tmp_path) -> None: + path = tmp_path / "cron.json" + store = CronStore(path=path) + path.write_text( + json.dumps( + [ + { + "name": "Newer", + "expression": "* * * * *", + "preset_name": "B", + "from_a_later_version": True, + } + ] + ), + encoding="utf-8", + ) + + assert store.reload_if_changed() is True + assert [j.name for j in store.jobs] == ["Newer"] + + +def test_save_leaves_no_temporary_file_behind(tmp_path) -> None: + store = CronStore(path=tmp_path / "cron.json") + store.add(job("Nightly")) + + assert [p.name for p in tmp_path.iterdir()] == ["cron.json"] + + +def test_save_never_leaves_the_file_unreadable(tmp_path) -> None: + """The reason for the atomic write: another instance is polling this. + + Rewriting a long file with a short one is where a plain write is briefly + truncated on disk; os.replace has no such window. + """ + path = tmp_path / "cron.json" + store = CronStore(path=path) + for index in range(20): + store.jobs.append(job(f"Job {index}")) + store.save() + + store.jobs = [job("Only one")] + store.save() + + assert len(json.loads(path.read_text(encoding="utf-8"))) == 1 + + +def test_presets_reload_the_same_way(tmp_path) -> None: + path = tmp_path / "presets.json" + here, elsewhere = PresetStore(path=path), PresetStore(path=path) + elsewhere.add(Preset(name="Deploy", lines=["make deploy"], target="new_tab")) + + assert here.reload_if_changed() is True + assert here.presets[-1].name == "Deploy" diff --git a/tests/test_cron_editor.py b/tests/test_cron_editor.py index fbd3873..9dedad6 100644 --- a/tests/test_cron_editor.py +++ b/tests/test_cron_editor.py @@ -240,3 +240,45 @@ def test_clearing_the_group_makes_it_ungrouped(qtbot, tmp_path: Path) -> None: dialog._save_current() assert cron_store.jobs[0].group is None + + +def test_the_open_editor_holds_off_a_reload(qtbot, tmp_path: Path) -> None: + """The form addresses jobs by index, and a modal dialog still runs the + event loop - so a poll landing mid-edit would save onto the wrong job.""" + cron_store, preset_store = make_stores( + tmp_path, [CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup")] + ) + cron_store.save() + dialog = CronEditorDialog(cron_store, preset_store) + qtbot.addWidget(dialog) + + elsewhere = CronStore(path=tmp_path / "cron.json") + elsewhere.jobs.insert( + 0, CronJob(name="Sneaked In", expression="* * * * *", preset_name="Backup") + ) + elsewhere.save() + + assert cron_store.reload_if_changed() is False + assert [job.name for job in cron_store.jobs] == ["Nightly"] + + dialog.reject() + + assert cron_store.reload_if_changed() is True + assert [job.name for job in cron_store.jobs] == ["Sneaked In", "Nightly"] + + +def test_the_open_editor_holds_off_a_preset_reload_too(qtbot, tmp_path: Path) -> None: + """Its Macro combo is built once, so the preset list must hold still too.""" + cron_store, preset_store = make_stores(tmp_path) + preset_store.save() + dialog = CronEditorDialog(cron_store, preset_store) + qtbot.addWidget(dialog) + + elsewhere = PresetStore(path=tmp_path / "presets.json") + elsewhere.add(Preset(name="Deploy", lines=["make deploy"], target="new_tab")) + + assert preset_store.reload_if_changed() is False + + dialog.reject() + + assert preset_store.reload_if_changed() is True diff --git a/tests/test_cron_scheduler.py b/tests/test_cron_scheduler.py index 73dbd5b..1b2736f 100644 --- a/tests/test_cron_scheduler.py +++ b/tests/test_cron_scheduler.py @@ -194,3 +194,85 @@ def test_run_now_ignores_the_schedule(tmp_path) -> None: assert scheduler.run_now(job) is True assert tabs.fed == [(tabs.open_tabs[0], ["echo hi"])] + + +def test_a_job_added_by_another_instance_fires_without_a_restart(tmp_path) -> None: + """The whole point of polling: two windows open, job added in the other.""" + cron_path, preset_path = tmp_path / "cron.json", tmp_path / "presets.json" + preset_store = PresetStore(path=preset_path) + preset_store.presets = [Preset(name="Backup", lines=["x"], target="new_tab")] + preset_store.save() + scheduler = CronScheduler(CronStore(path=cron_path), preset_store, FakeTabs()) + + elsewhere = CronStore(path=cron_path) + elsewhere.add(CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup")) + + scheduler.refresh_config() + + assert scheduler.run_due_jobs(at("2026-08-17 02:00")) == ["Nightly"] + + +def test_a_macro_added_by_another_instance_arrives_with_its_job(tmp_path) -> None: + """A job is useless here without the Macro it names, so both are polled.""" + cron_path, preset_path = tmp_path / "cron.json", tmp_path / "presets.json" + preset_store = PresetStore(path=preset_path) + preset_store.presets = [] + preset_store.save() + cron_store = CronStore(path=cron_path) + scheduler = CronScheduler(cron_store, preset_store, FakeTabs()) + failures = [] + scheduler.job_failed.connect(lambda name, why: failures.append((name, why))) + + other_presets = PresetStore(path=preset_path) + other_presets.presets = [Preset(name="Backup", lines=["x"], target="new_tab")] + other_presets.save() + other_cron = CronStore(path=cron_path) + other_cron.add( + CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup") + ) + + scheduler.refresh_config() + + assert scheduler.run_due_jobs(at("2026-08-17 02:00")) == ["Nightly"] + assert failures == [] + + +def test_a_job_deleted_elsewhere_stops_firing(tmp_path) -> None: + cron_path = tmp_path / "cron.json" + preset_store = PresetStore(path=tmp_path / "presets.json") + preset_store.presets = [Preset(name="Backup", lines=["x"], target="new_tab")] + cron_store = CronStore(path=cron_path) + cron_store.add(CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup")) + scheduler = CronScheduler(cron_store, preset_store, FakeTabs()) + + elsewhere = CronStore(path=cron_path) + elsewhere.delete(0) + + scheduler.refresh_config() + + assert scheduler.run_due_jobs(at("2026-08-17 02:00")) == [] + + +def test_a_reload_keeps_a_running_job_pointed_at_its_own_tab(tmp_path) -> None: + """Tabs are keyed by job name, so re-reading the file must not orphan one.""" + cron_path = tmp_path / "cron.json" + preset_store = PresetStore(path=tmp_path / "presets.json") + preset_store.presets = [Preset(name="Backup", lines=["x"], target="new_tab")] + cron_store = CronStore(path=cron_path) + cron_store.add(CronJob(name="Nightly", expression="0 2 * * *", preset_name="Backup")) + tabs = FakeTabs() + scheduler = CronScheduler(cron_store, preset_store, tabs) + + scheduler.run_due_jobs(at("2026-08-17 02:00")) + first_tab = tabs.open_tabs[0] + + # Another instance re-saves the same job with a changed schedule. + elsewhere = CronStore(path=cron_path) + elsewhere.update( + 0, CronJob(name="Nightly", expression="0 3 * * *", preset_name="Backup") + ) + scheduler.refresh_config() + scheduler.run_due_jobs(at("2026-08-18 03:00")) + + assert tabs.open_tabs == [first_tab] + assert [terminal for terminal, _ in tabs.fed] == [first_tab, first_tab] diff --git a/tests/test_preset_editor.py b/tests/test_preset_editor.py index 9f9e017..254bc1b 100644 --- a/tests/test_preset_editor.py +++ b/tests/test_preset_editor.py @@ -450,3 +450,27 @@ def test_the_new_order_is_saved_and_shown_in_the_menu(qtbot, tmp_path: Path) -> menu = MacrosMenu(store, tabs) listed = [a.text() for a in menu.actions()][:2] assert listed == ["Second", "First"] + + +def test_the_open_editor_holds_off_a_reload(qtbot, tmp_path) -> None: + """Presets are addressed by index here, so the list must not be swapped + out by a poll while the form is open.""" + store = PresetStore(path=tmp_path / "presets.json") + store.presets = [Preset(name="Build", lines=["make"], target="active")] + store.save() + dialog = PresetEditorDialog(store, category=CATEGORY_COMMANDS) + qtbot.addWidget(dialog) + + elsewhere = PresetStore(path=tmp_path / "presets.json") + elsewhere.presets.insert( + 0, Preset(name="Sneaked In", lines=["x"], target="active") + ) + elsewhere.save() + + assert store.reload_if_changed() is False + assert [p.name for p in store.presets] == ["Build"] + + dialog.reject() + + assert store.reload_if_changed() is True + assert [p.name for p in store.presets] == ["Sneaked In", "Build"]