From 6f6f83f71eccab60294ceac21cc244ed7eb94d3a Mon Sep 17 00:00:00 2001 From: Jer Miller Date: Wed, 29 Jul 2026 22:53:46 -0600 Subject: [PATCH] remove(schedules): drop the retired granola sync entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Granola import retirement left owners' own `sync:granola` entry in config/schedules.json. Nothing stops it: scheduler.load_config validates entry shape only and never checks that a `sync:` backend exists, and the supervisor records last_run on every completion including non-zero exits, so the entry re-fires at each hour boundary and fails every time with "Unknown sync backend: granola". There is no in-product way off it — `journal schedule` is display-only and the settings sync POST that could have toggled it is gone with the retirement. Add maint 009 to remove exactly that one key. The predicate matches only the literal name `sync:granola` and only when the entry has no cmd or its cmd invokes the granola importer sync, accepting the current, pre-maint-007, and `--sync=granola` forms; anything else under that key is preserved with a warning. Removal ignores `enabled`, since a disabled entry for a source that no longer exists is still dead config. The only write routes through schedule_config.remove_schedule_entry, the registered owner for config/schedules.json. Read-side malformed input (missing, empty, non-JSON, non-dict) sets a skip reason and exits 0; errors and exit 1 are reserved for lock and write failures, which MAINT_RETRY_ON_NEXT_START retries on the next supervisor start. Sibling schedule entries, health/scheduler.json, and the granola sync-state file are untouched. Bumps the tests/test_maint.py opt-in inventory guard to carry the new retry-only task. Co-Authored-By: Claude Opus 5 (1M context) --- .../maint/009_remove_granola_sync_schedule.py | 153 ++++++++ tests/test_maint.py | 11 +- ..._maint_009_remove_granola_sync_schedule.py | 367 ++++++++++++++++++ 3 files changed, 527 insertions(+), 4 deletions(-) create mode 100644 solstone/apps/sol/maint/009_remove_granola_sync_schedule.py create mode 100644 tests/test_maint_009_remove_granola_sync_schedule.py diff --git a/solstone/apps/sol/maint/009_remove_granola_sync_schedule.py b/solstone/apps/sol/maint/009_remove_granola_sync_schedule.py new file mode 100644 index 000000000..982163683 --- /dev/null +++ b/solstone/apps/sol/maint/009_remove_granola_sync_schedule.py @@ -0,0 +1,153 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright (c) 2026 sol pbc + +"""Remove the retired sync:granola schedule entry.""" + +from __future__ import annotations + +import argparse +import json +import sys +from dataclasses import dataclass, field + +from solstone.think.journal_io.errors import LockTimeout, MalformedDataError +from solstone.think.schedule_config import get_schedules_path, remove_schedule_entry +from solstone.think.utils import setup_cli + +MAINT_RETRY_ON_NEXT_START = True + +GRANOLA_SCHEDULE_NAME = "sync:granola" +GRANOLA_BACKEND = "granola" +SYNC_EQUALS_FORM = f"--sync={GRANOLA_BACKEND}" +SYNC_SURFACES = {"journal", "sol"} +SYNC_COMMANDS = {"import", "importer"} + + +@dataclass +class MigrationSummary: + removed: int = 0 + removed_names: list[str] = field(default_factory=list) + preserved: int = 0 + preserved_names: list[str] = field(default_factory=list) + errors: int = 0 + skipped_reason: str | None = None + + +def _is_retired_granola_sync(value: object) -> bool: + if not isinstance(value, dict): + return False + + if "cmd" not in value: + return True + + cmd = value["cmd"] + if not isinstance(cmd, list) or not all(isinstance(part, str) for part in cmd): + return False + if len(cmd) < 2: + return False + if cmd[0] not in SYNC_SURFACES or cmd[1] not in SYNC_COMMANDS: + return False + + for index, part in enumerate(cmd): + if part == SYNC_EQUALS_FORM: + return True + if ( + part == "--sync" + and index + 1 < len(cmd) + and cmd[index + 1] == GRANOLA_BACKEND + ): + return True + + return False + + +def run_migration(*, dry_run: bool) -> MigrationSummary: + summary = MigrationSummary() + schedules_path = get_schedules_path() + + # Read explicitly so missing, empty, and unparseable inputs keep distinct + # skip reasons; write-side MalformedDataError stays confined to the owner + # mutation helper below. + if not schedules_path.exists(): + summary.skipped_reason = "no file" + return summary + + try: + raw_bytes = schedules_path.read_bytes() + except OSError as exc: + summary.errors += 1 + print(f"[ERROR] read failed: {schedules_path}: {exc}") + return summary + + if not raw_bytes.strip(): + summary.skipped_reason = "empty file" + return summary + + try: + raw = json.loads(raw_bytes) + except json.JSONDecodeError: + summary.skipped_reason = "unparseable" + return summary + + if not isinstance(raw, dict): + summary.skipped_reason = "unparseable" + return summary + + name = GRANOLA_SCHEDULE_NAME + if name not in raw: + return summary + + if not _is_retired_granola_sync(raw.get(name)): + summary.preserved += 1 + summary.preserved_names.append(name) + print(f"WARNING: preserving owner-divergent schedule entry {name}") + return summary + + print(f"{'[DRY-RUN] ' if dry_run else ''}remove {name}") + summary.removed += 1 + summary.removed_names.append(name) + + if dry_run: + return summary + + try: + remove_schedule_entry(name) + except (OSError, MalformedDataError, LockTimeout) as exc: + summary.errors += 1 + print(f"[ERROR] remove failed: {name}: {exc}") + + return summary + + +def _print_summary(summary: MigrationSummary) -> None: + print("Summary") + print(f" removed: {summary.removed}") + print(f" preserved: {summary.preserved}") + print(f" errors: {summary.errors}") + if summary.skipped_reason is not None: + print(f" skipped: {summary.skipped_reason}") + for name in summary.removed_names: + print(f" removed_name: {name}") + for name in summary.preserved_names: + print(f" preserved_name: {name}") + + +def main() -> None: + parser = argparse.ArgumentParser( + description="Remove the retired sync:granola schedule entry." + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="Preview retired schedule removal without writing files.", + ) + args = setup_cli(parser) + + summary = run_migration(dry_run=args.dry_run) + _print_summary(summary) + if summary.errors: + sys.exit(1) + + +if __name__ == "__main__": + main() diff --git a/tests/test_maint.py b/tests/test_maint.py index 6842a7604..de6cad053 100644 --- a/tests/test_maint.py +++ b/tests/test_maint.py @@ -365,7 +365,10 @@ class TestRunPendingTasks: "thinking:001_migrate_provider_install_state", "thinking:002_pin_google_model_aliases", } - retry_migrations = {"sol:008_migrate_provider_check_schedule"} + retry_migrations = { + "sol:008_migrate_provider_check_schedule", + "sol:009_remove_granola_sync_schedule", + } existing = [ task for task in tasks @@ -385,9 +388,9 @@ class TestRunPendingTasks: assert all(task.blocks_supervisor_start is True for task in migration) retry_only = [task for task in tasks if task.qualified_name in retry_migrations] - assert len(retry_only) == 1 - assert retry_only[0].retry_on_next_start is True - assert retry_only[0].blocks_supervisor_start is False + assert len(retry_only) == 2 + assert all(task.retry_on_next_start is True for task in retry_only) + assert all(task.blocks_supervisor_start is False for task in retry_only) class TestRunTask: diff --git a/tests/test_maint_009_remove_granola_sync_schedule.py b/tests/test_maint_009_remove_granola_sync_schedule.py new file mode 100644 index 000000000..efdc3e910 --- /dev/null +++ b/tests/test_maint_009_remove_granola_sync_schedule.py @@ -0,0 +1,367 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright (c) 2026 sol pbc + +from __future__ import annotations + +import importlib +import json +import sys +from pathlib import Path +from typing import Callable + +import pytest + +from solstone.think.journal_io.errors import LockTimeout + +mod = importlib.import_module( + "solstone.apps.sol.maint.009_remove_granola_sync_schedule" +) + + +@pytest.fixture(autouse=True) +def _use_tmp_journal(tmp_path, monkeypatch): + monkeypatch.setenv("SOLSTONE_JOURNAL", str(tmp_path)) + + +def _write_schedules(journal: Path, data: object) -> Path: + config_dir = journal / "config" + config_dir.mkdir(parents=True, exist_ok=True) + schedules_path = config_dir / "schedules.json" + schedules_path.write_text(json.dumps(data, indent=2), encoding="utf-8") + return schedules_path + + +def _read_schedules(path: Path) -> dict: + return json.loads(path.read_text(encoding="utf-8")) + + +def _current_granola_entry(*, enabled: bool = True) -> dict: + return { + "cmd": ["journal", "importer", "--sync", "granola", "--save"], + "every": "hourly", + "enabled": enabled, + } + + +def test_removes_enabled_current_granola_schedule(tmp_path): + schedules_path = _write_schedules( + tmp_path, + {"sync:granola": _current_granola_entry(enabled=True)}, + ) + + summary = mod.run_migration(dry_run=False) + + assert summary.removed == 1 + assert summary.removed_names == ["sync:granola"] + assert summary.errors == 0 + assert "sync:granola" not in _read_schedules(schedules_path) + + +def test_removes_disabled_current_granola_schedule(tmp_path): + schedules_path = _write_schedules( + tmp_path, + {"sync:granola": _current_granola_entry(enabled=False)}, + ) + + summary = mod.run_migration(dry_run=False) + + assert summary.removed == 1 + assert summary.removed_names == ["sync:granola"] + assert summary.errors == 0 + assert "sync:granola" not in _read_schedules(schedules_path) + + +def test_removes_only_granola_and_preserves_sibling_values_and_order(tmp_path): + sync_plaud = { + "cmd": ["journal", "importer", "--sync", "plaud", "--save"], + "every": "hourly", + "enabled": True, + } + sync_obsidian = { + "cmd": ["journal", "importer", "--sync", "obsidian", "--save"], + "every": "hourly", + "enabled": False, + } + heartbeat = { + "cmd": ["journal", "heartbeat"], + "every": "daily", + "enabled": True, + "max_runtime": "10m", + } + weekly_agents = { + "cmd": ["journal", "think", "--weekly", "-v"], + "every": "weekly", + "enabled": True, + "max_runtime": "30m", + } + initial = { + "sync:plaud": sync_plaud, + "sync:granola": _current_granola_entry(enabled=True), + "sync:obsidian": sync_obsidian, + "heartbeat": heartbeat, + "weekly-agents": weekly_agents, + "daily_time": "03:17", + "weekly_time": "04:21", + } + schedules_path = _write_schedules(tmp_path, initial) + expected = { + name: value for name, value in initial.items() if name != "sync:granola" + } + + summary = mod.run_migration(dry_run=False) + data = _read_schedules(schedules_path) + + assert summary.removed == 1 + assert summary.errors == 0 + assert data == expected + assert list(data) == list(expected) + + +def test_removes_pre_007_sol_import_granola_schedule(tmp_path): + schedules_path = _write_schedules( + tmp_path, + { + "sync:granola": { + "cmd": ["sol", "import", "--sync", "granola", "--save"], + "every": "hourly", + "enabled": True, + } + }, + ) + + summary = mod.run_migration(dry_run=False) + + assert summary.removed == 1 + assert summary.removed_names == ["sync:granola"] + assert summary.errors == 0 + assert "sync:granola" not in _read_schedules(schedules_path) + + +def test_removes_equals_form_granola_schedule(tmp_path): + schedules_path = _write_schedules( + tmp_path, + { + "sync:granola": { + "cmd": ["journal", "importer", "--sync=granola", "--save"], + "every": "hourly", + "enabled": True, + } + }, + ) + + summary = mod.run_migration(dry_run=False) + + assert summary.removed == 1 + assert summary.removed_names == ["sync:granola"] + assert summary.errors == 0 + assert "sync:granola" not in _read_schedules(schedules_path) + + +def test_removes_granola_entry_without_cmd(tmp_path): + schedules_path = _write_schedules( + tmp_path, + {"sync:granola": {"every": "hourly", "enabled": True}}, + ) + + summary = mod.run_migration(dry_run=False) + + assert summary.removed == 1 + assert summary.removed_names == ["sync:granola"] + assert summary.errors == 0 + assert "sync:granola" not in _read_schedules(schedules_path) + + +def test_preserves_non_dict_granola_entry_and_prints_warning(tmp_path, capsys): + initial = {"sync:granola": "journal importer --sync granola"} + schedules_path = _write_schedules(tmp_path, initial) + + summary = mod.run_migration(dry_run=False) + captured = capsys.readouterr() + + assert summary.removed == 0 + assert summary.preserved == 1 + assert summary.preserved_names == ["sync:granola"] + assert summary.errors == 0 + assert _read_schedules(schedules_path) == initial + assert "WARNING" in captured.out + assert "sync:granola" in captured.out + + +def test_preserves_other_backend_granola_key_and_prints_warning(tmp_path, capsys): + legacy_entry = { + "cmd": ["journal", "importer", "--sync", "plaud", "--save"], + "every": "hourly", + "enabled": True, + } + schedules_path = _write_schedules(tmp_path, {"sync:granola": legacy_entry}) + + summary = mod.run_migration(dry_run=False) + captured = capsys.readouterr() + + assert summary.removed == 0 + assert summary.preserved == 1 + assert summary.preserved_names == ["sync:granola"] + assert summary.errors == 0 + assert _read_schedules(schedules_path) == {"sync:granola": legacy_entry} + assert "WARNING" in captured.out + assert "sync:granola" in captured.out + + +def test_dry_run_reports_would_remove_and_preserves_file_bytes_and_mtime(tmp_path): + schedules_path = _write_schedules( + tmp_path, + {"sync:granola": _current_granola_entry(enabled=True)}, + ) + before_bytes = schedules_path.read_bytes() + before_mtime_ns = schedules_path.stat().st_mtime_ns + + summary = mod.run_migration(dry_run=True) + + assert summary.removed == 1 + assert summary.removed_names == ["sync:granola"] + assert summary.errors == 0 + assert schedules_path.read_bytes() == before_bytes + assert schedules_path.stat().st_mtime_ns == before_mtime_ns + + +def test_second_run_is_noop_and_does_not_rewrite_schedules_file(tmp_path): + schedules_path = _write_schedules( + tmp_path, + {"sync:granola": _current_granola_entry(enabled=True)}, + ) + + first = mod.run_migration(dry_run=False) + after_first_bytes = schedules_path.read_bytes() + after_first_mtime_ns = schedules_path.stat().st_mtime_ns + second = mod.run_migration(dry_run=False) + + assert first.removed == 1 + assert first.errors == 0 + assert second.removed == 0 + assert second.removed_names == [] + assert second.errors == 0 + assert schedules_path.read_bytes() == after_first_bytes + assert schedules_path.stat().st_mtime_ns == after_first_mtime_ns + + +@pytest.mark.parametrize( + ("payload", "expected_skipped_reason"), + [ + pytest.param(None, "no file", id="missing-file"), + pytest.param(b"", "empty file", id="empty-file"), + pytest.param(b"{not json", "unparseable", id="non-json"), + pytest.param(b"[]", "unparseable", id="non-dict"), + ], +) +def test_read_side_bad_or_missing_schedules_skip_without_errors_and_cli_exits_zero( + tmp_path, + monkeypatch, + payload, + expected_skipped_reason, +): + if payload is not None: + config_dir = tmp_path / "config" + config_dir.mkdir(parents=True, exist_ok=True) + (config_dir / "schedules.json").write_bytes(payload) + + summary = mod.run_migration(dry_run=False) + + assert summary.skipped_reason == expected_skipped_reason + assert summary.errors == 0 + + monkeypatch.setattr(sys, "argv", ["maint-009"]) + mod.main() + + +@pytest.mark.parametrize( + "exc_factory", + [ + pytest.param(lambda _path: OSError("boom"), id="oserror"), + pytest.param(lambda path: LockTimeout(path, 1.0), id="lock-timeout"), + ], +) +def test_remove_failure_records_error_preserves_bytes_and_cli_exits_one( + tmp_path, + monkeypatch, + exc_factory: Callable[[Path], Exception], +): + schedules_path = _write_schedules( + tmp_path, + {"sync:granola": _current_granola_entry(enabled=True)}, + ) + before_bytes = schedules_path.read_bytes() + + def _raise_remove(_name: str) -> None: + raise exc_factory(schedules_path) + + monkeypatch.setattr(mod, "remove_schedule_entry", _raise_remove) + + summary = mod.run_migration(dry_run=False) + + assert summary.removed == 1 + assert summary.removed_names == ["sync:granola"] + assert summary.errors == 1 + assert schedules_path.read_bytes() == before_bytes + + monkeypatch.setattr(sys, "argv", ["maint-009"]) + with pytest.raises(SystemExit) as exc_info: + mod.main() + + assert exc_info.value.code == 1 + assert schedules_path.read_bytes() == before_bytes + + +def test_task_opt_ins_are_retry_only(): + assert mod.MAINT_RETRY_ON_NEXT_START is True + assert getattr(mod, "MAINT_BLOCKS_SUPERVISOR_START", False) is False + + +def test_health_scheduler_state_is_untouched(tmp_path): + _write_schedules( + tmp_path, + {"sync:granola": _current_granola_entry(enabled=True)}, + ) + health_dir = tmp_path / "health" + health_dir.mkdir(parents=True, exist_ok=True) + state_path = health_dir / "scheduler.json" + state_path.write_text( + json.dumps({"sync:granola": {"last_run": 123}}, indent=2), + encoding="utf-8", + ) + before_bytes = state_path.read_bytes() + + summary = mod.run_migration(dry_run=False) + + assert summary.removed == 1 + assert summary.errors == 0 + assert state_path.read_bytes() == before_bytes + + +def test_only_exact_sync_granola_key_is_considered_and_other_backends_are_preserved( + tmp_path, +): + assert ( + mod._is_retired_granola_sync( + {"cmd": ["journal", "importer", "--sync", "plaud", "--save"]} + ) + is False + ) + granola_looking = { + "cmd": ["journal", "importer", "--sync", "granola", "--save"], + "every": "hourly", + "enabled": True, + } + initial = { + "sync:plaud": granola_looking, + "sync:granola-copy": granola_looking, + } + schedules_path = _write_schedules(tmp_path, initial) + before_bytes = schedules_path.read_bytes() + + summary = mod.run_migration(dry_run=False) + + assert summary.removed == 0 + assert summary.preserved == 0 + assert summary.errors == 0 + assert schedules_path.read_bytes() == before_bytes + assert _read_schedules(schedules_path) == initial -- 2.51.2