Skip to content

Commit 13532ce

Browse files
rigidlabclaude
andauthored
Feat/cron config polling (#7)
* 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8UgLA68nRCYg3qFTyW8ms * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8UgLA68nRCYg3qFTyW8ms * 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M8UgLA68nRCYg3qFTyW8ms --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent bbaa3c4 commit 13532ce

12 files changed

Lines changed: 588 additions & 40 deletions

‎SPEC.md‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -815,14 +815,67 @@ Four decisions, each taken over a plausible alternative:
815815
the preset menus rather than growing its own copy of the ownership
816816
handling those exist for.
817817

818+
### Phase 4s.1 - Config shared between instances ✅ done
819+
Two qtxterm windows are two processes over one `cron.json` and one
820+
`presets.json`. Each loaded its file once at launch, so a job added in one
821+
was invisible to the other until restart - and the next save from the stale
822+
window wrote its old list over the new one, losing the job silently.
823+
824+
- [x] **Polled once a minute, not watched.** The scheduler's tick already
825+
runs every second, so checking on the minute costs a `stat()` and *no
826+
extra wakeup*. A `QFileSystemWatcher` costs a worker thread, wakes on
827+
every change in the config directory, and stops watching a file that
828+
is *replaced* rather than modified - which is exactly what the atomic
829+
save below does. A minute is also cron's own resolution: there is
830+
nothing to do with a job noticed sooner than it could first fire.
831+
- [x] **Stamped, so an unchanged file is never re-parsed.** `(mtime_ns,
832+
size)`, compared before reading. Reloading unconditionally would emit
833+
`changed` every minute and rebuild every menu, including one the user
834+
has open.
835+
- [x] **Presets are polled too, not just jobs.** A job added elsewhere
836+
normally arrives with the Macro it names, and a job whose Macro has
837+
not been re-read fails every single firing with "no Macro named ...
838+
any more".
839+
- [x] **Atomic save**, via a sibling temp file and `os.replace`. The reader
840+
is another process polling this exact path; a plain write is visible in
841+
its truncated middle.
842+
- [x] **Reloads are suspended while an editor is open.** Both editors address
843+
entries by index, and an `exec()` dialog still runs the event loop - a
844+
poll landing mid-edit would shift the list under the open form and save
845+
the user's changes onto whichever entry inherited that index. Counted,
846+
not a flag, since two editors can be open over one store.
847+
- [x] **A bad read changes nothing and is retried.** Invalid JSON - a
848+
hand-edit in progress, a half-synced cloud drive, an older qtxterm
849+
without the atomic save - leaves the in-memory list alone and does not
850+
advance the stamp. Raising out of a timer would take down the window
851+
over a file that is very likely fine a second later. A *deleted* file
852+
is treated the same way: the presets do not evaporate mid-session.
853+
854+
Two things this deliberately does not fix:
855+
856+
- **The clobber window is narrowed, not closed.** Saves still write the whole
857+
file, so an edit made in two windows inside the same minute still resolves
858+
last-writer-wins. Closing it properly means merging per entry, which needs
859+
stable ids the files do not have.
860+
- **Both instances still fire the same job**, each into its own tab, since
861+
each runs its own scheduler. Cron is per-instance by design. If that ever
862+
needs to change, the fix is a single-instance lock at launch, not a
863+
fourth thing for the poll to do.
864+
865+
Implementation note: `config_store.py` holds this once and both stores
866+
inherit it - `PresetStore` and `CronStore` were already the same problem
867+
twice (a list of dataclasses, one JSON file, editors addressing entries by
868+
index), and the file handling is now the part they literally share.
869+
818870
Implementation notes:
819871

820872
- `cron.py` is schedules and storage and knows nothing about terminals;
821873
`cron_scheduler.py` owns the tab-per-job and the firing. Splitting them is
822874
what let the whole expression layer be tested without a Qt widget.
823875
- The scheduler ticks every second and acts only when the *minute* changes.
824876
A 60s timer drifts against the wall clock and skips a minute whenever the
825-
machine sleeps.
877+
machine sleeps. On each minute change it re-reads the config first, then
878+
fires - see Phase 4s.1.
826879
- `next_run()` walks minute by minute but skips whole days that cannot match,
827880
and gives up after four years - "0 0 31 2 *" parses fine and can never
828881
happen, and the alternative to a bound is a UI that hangs.

‎src/qtxterm/assets/USAGE.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,14 +364,19 @@ that menu, the same way Macros do - useful once you have a job per feed per
364364
session. Ungrouped jobs stay at the top level. **Run Now** in the editor runs a job
365365
immediately, which beats waiting until 2am to find out whether it works.
366366

367-
Two things to know:
367+
Three things to know:
368368

369369
- **Jobs only run while qtxterm is open**, and nothing missed while it was
370370
closed is caught up on. Launching after a weekend does not fire a burst of
371371
overdue jobs. For work that must happen whether or not you're at the
372372
machine, use the system's own scheduler.
373373
- **A job names its Macro.** Rename or delete that Macro and the job says so
374374
in the status bar rather than running something else.
375+
- **Two qtxterm windows share one set of jobs.** Add or edit a job in one and
376+
the other picks it up within a minute, no restart needed. Note that each
377+
window runs its own schedule, so a job fires once per open window, each
378+
into its own tab. If that isn't what you want, keep one window open, or
379+
turn the job off in the others.
375380

376381
## Ordering
377382

‎src/qtxterm/config_store.py‎

Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
"""Shared file handling for the JSON config stores.
2+
3+
`PresetStore` and `CronStore` are the same problem twice: a list of
4+
dataclasses in one JSON file under the user's config dir, edited by dialogs
5+
that address entries by index. This holds the parts that are identical - the
6+
atomic write, and picking up a file a *second qtxterm instance* has written.
7+
8+
On the second instance: each process loads its config once at launch and
9+
keeps it in memory, so without this a job added in one window is invisible to
10+
the other, and the next save from the stale window writes its old list over
11+
the new one. Polling closes most of that gap - see `reload_if_changed`.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
import json
17+
import os
18+
from contextlib import contextmanager
19+
from pathlib import Path
20+
21+
from PySide6.QtCore import QObject, Signal
22+
23+
# What we compare to decide the file changed under us. mtime alone is too
24+
# coarse on filesystems with a 1-2s timestamp granularity (FAT, some network
25+
# shares), where two writes in the same second look identical; the size
26+
# catches the common case of those two writes differing in length.
27+
_Stamp = tuple[int, int] | None
28+
29+
30+
class ConfigStore(QObject):
31+
"""A JSON list on disk, reloaded when another process rewrites it.
32+
33+
Subclasses own the shape of the payload (`_apply_payload` /
34+
`_build_payload`) and what an absent file means (`_apply_missing`); this
35+
class owns the bytes.
36+
"""
37+
38+
changed = Signal()
39+
40+
def __init__(self, path: Path) -> None:
41+
super().__init__()
42+
self.path = path
43+
self._stamp: _Stamp = None
44+
self._suspended = 0
45+
46+
# -- subclass hooks ----------------------------------------------------
47+
48+
def _apply_payload(self, raw: list) -> None:
49+
raise NotImplementedError
50+
51+
def _build_payload(self) -> list:
52+
raise NotImplementedError
53+
54+
def _apply_missing(self) -> None:
55+
"""Called by load() when the file does not exist yet."""
56+
raise NotImplementedError
57+
58+
# -- file handling -----------------------------------------------------
59+
60+
def load(self) -> None:
61+
if not self.path.exists():
62+
self._stamp = None
63+
self._apply_missing()
64+
return
65+
self._apply_payload(json.loads(self.path.read_text(encoding="utf-8")))
66+
self._stamp = self._current_stamp()
67+
68+
def save(self) -> None:
69+
"""Write the whole file, atomically, and announce it.
70+
71+
Atomically because another instance may be polling this exact file:
72+
a plain write is visible in its truncated middle, and the reader gets
73+
a JSONDecodeError instead of a config. `os.replace` means a reader
74+
sees either the old file or the new one.
75+
"""
76+
self.path.parent.mkdir(parents=True, exist_ok=True)
77+
text = json.dumps(self._build_payload(), indent=2)
78+
temp = self.path.with_name(f"{self.path.name}.{os.getpid()}.tmp")
79+
try:
80+
temp.write_text(text, encoding="utf-8")
81+
os.replace(temp, self.path)
82+
except OSError:
83+
temp.unlink(missing_ok=True)
84+
raise
85+
self._stamp = self._current_stamp()
86+
self.changed.emit()
87+
88+
def reload_if_changed(self) -> bool:
89+
"""Re-read if another process has rewritten the file. Did we reload?
90+
91+
Cheap enough to call on a timer: a stat, and a parse only when the
92+
stamp moved. Callers get `changed` exactly as if the edit had been
93+
made here, so every menu and sidebar already knows what to do with it.
94+
95+
Never raises. A file that is unreadable *now* - mid-write by an older
96+
qtxterm without the atomic save, half-synced by a cloud drive, or
97+
hand-edited into invalid JSON - leaves the in-memory list alone and
98+
is retried on the next poll. Throwing from a timer would take out the
99+
window over a file that is very likely fine a second later.
100+
"""
101+
if self._suspended:
102+
return False
103+
stamp = self._current_stamp()
104+
if stamp == self._stamp:
105+
return False
106+
if stamp is None:
107+
# Deleted out from under us. Keeping what we have is the kinder
108+
# reading: the file coming back is a reload, and until then the
109+
# user's presets do not evaporate mid-session.
110+
self._stamp = None
111+
return False
112+
try:
113+
raw = json.loads(self.path.read_text(encoding="utf-8"))
114+
except (OSError, ValueError):
115+
return False
116+
# Stamped only on success, so a failed read is retried rather than
117+
# remembered as done.
118+
self._apply_payload(raw)
119+
self._stamp = stamp
120+
self.changed.emit()
121+
return True
122+
123+
def suspend_reload(self) -> None:
124+
"""Hold off reloads while an editor is open over this store.
125+
126+
The editors address entries by index (`update(index, ...)`), and a
127+
modal dialog still runs the event loop, so a poll landing mid-edit
128+
would shift the list under the open form and save the user's changes
129+
onto whichever entry inherited that index. Counted rather than a
130+
flag, because more than one editor can be open over one store.
131+
"""
132+
self._suspended += 1
133+
134+
def resume_reload(self) -> None:
135+
self._suspended = max(0, self._suspended - 1)
136+
137+
@contextmanager
138+
def reload_suspended(self):
139+
self.suspend_reload()
140+
try:
141+
yield
142+
finally:
143+
self.resume_reload()
144+
145+
def _current_stamp(self) -> _Stamp:
146+
try:
147+
info = self.path.stat()
148+
except OSError:
149+
return None
150+
return (info.st_mtime_ns, info.st_size)

‎src/qtxterm/cron.py‎

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@
1616
from __future__ import annotations
1717

1818
import dataclasses
19-
import json
2019
from datetime import datetime, timedelta
2120
from pathlib import Path
2221

2322
import platformdirs
24-
from PySide6.QtCore import QObject, Signal
23+
24+
from qtxterm.config_store import ConfigStore
2525

2626
# (name, low, high) per field, in the order they are written.
2727
_FIELDS = [
@@ -222,34 +222,27 @@ def default_cron_path() -> Path:
222222
return Path(platformdirs.user_config_dir("qtxterm", appauthor=False)) / "cron.json"
223223

224224

225-
class CronStore(QObject):
225+
class CronStore(ConfigStore):
226226
"""Loads/saves cron jobs as JSON, the same shape PresetStore uses."""
227227

228-
changed = Signal()
229-
230228
def __init__(self, path: Path | None = None) -> None:
231-
super().__init__()
232-
self.path = path or default_cron_path()
229+
super().__init__(path or default_cron_path())
233230
self.jobs: list[CronJob] = []
234231
self.load()
235232

236-
def load(self) -> None:
237-
if not self.path.exists():
238-
self.jobs = []
239-
return
240-
raw = json.loads(self.path.read_text(encoding="utf-8"))
233+
def _apply_missing(self) -> None:
234+
self.jobs = []
235+
236+
def _apply_payload(self, raw: list) -> None:
241237
# Unknown keys are dropped rather than raising: a file written by a
242238
# later version should cost you a field, not the whole app.
243239
fields = {f.name for f in dataclasses.fields(CronJob)}
244240
self.jobs = [
245241
CronJob(**{k: v for k, v in item.items() if k in fields}) for item in raw
246242
]
247243

248-
def save(self) -> None:
249-
self.path.parent.mkdir(parents=True, exist_ok=True)
250-
payload = [dataclasses.asdict(job) for job in self.jobs]
251-
self.path.write_text(json.dumps(payload, indent=2), encoding="utf-8")
252-
self.changed.emit()
244+
def _build_payload(self) -> list:
245+
return [dataclasses.asdict(job) for job in self.jobs]
253246

254247
def add(self, job: CronJob) -> None:
255248
self.jobs.append(job)

‎src/qtxterm/cron_editor.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ def __init__(
6464
self._preset_store = preset_store
6565
self._scheduler = scheduler
6666
self._current_index: int | None = None
67+
# This form addresses jobs by index, so the lists behind it must not
68+
# be swapped out by a poll while it is open - see ConfigStore.
69+
self._store.suspend_reload()
70+
self._preset_store.suspend_reload()
71+
self.finished.connect(self._resume_reloads)
6772

6873
layout = QHBoxLayout(self)
6974

@@ -129,6 +134,10 @@ def __init__(
129134
if create_new:
130135
self._new_job()
131136

137+
def _resume_reloads(self) -> None:
138+
self._store.resume_reload()
139+
self._preset_store.resume_reload()
140+
132141
def _runnable_presets(self):
133142
"""Macros only.
134143

‎src/qtxterm/cron_scheduler.py‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ class CronScheduler(QObject):
2727
the app starts never fires, and nothing missed while it was closed is
2828
replayed. Otherwise launching after a weekend would open a burst of
2929
terminals before you had touched anything.
30+
31+
Config is re-read on the minute, so a job added in another instance is
32+
picked up here without a restart - see `refresh_config`.
3033
"""
3134

3235
job_fired = Signal(str)
@@ -73,8 +76,27 @@ def _on_tick(self) -> None:
7376
if minute == self._last_minute:
7477
return
7578
self._last_minute = minute
79+
self.refresh_config()
7680
self.run_due_jobs(minute)
7781

82+
def refresh_config(self) -> None:
83+
"""Pick up jobs and Macros written by another qtxterm instance.
84+
85+
Polled here rather than watched, for two reasons. This tick already
86+
happens every second, so checking on the minute costs a stat and no
87+
extra wakeup at all - where a QFileSystemWatcher costs a worker
88+
thread, and silently stops watching when a file is *replaced*, which
89+
is exactly what the atomic save in `ConfigStore` does. And a minute
90+
is the resolution cron works at anyway; there is nothing to be done
91+
with a job noticed sooner than the minute it could first fire.
92+
93+
Presets as well as jobs, because a job added elsewhere usually
94+
arrives with the Macro it runs, and a job whose Macro we have not
95+
re-read fails every firing with "no Macro named ... any more".
96+
"""
97+
self._preset_store.reload_if_changed()
98+
self._cron_store.reload_if_changed()
99+
78100
def run_due_jobs(self, minute: datetime) -> list[str]:
79101
"""Fire every enabled job whose schedule matches `minute`."""
80102
fired: list[str] = []

‎src/qtxterm/preset_editor.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@ def __init__(
121121
self.resize(620, 420)
122122
self._store = store
123123
self._current_index: int | None = None
124+
# Addresses presets by index, so a poll must not reorder the list
125+
# under the open form - see ConfigStore.
126+
self._store.suspend_reload()
127+
self.finished.connect(lambda _: self._store.resume_reload())
124128
self._is_command = category == CATEGORY_COMMANDS
125129
self._is_macro = category == CATEGORY_MACROS
126130
self._is_selection = category == CATEGORY_SELECTION

0 commit comments

Comments
 (0)