diff --git a/CHANGELOG.md b/CHANGELOG.md index d3f2c4c..14b91b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,19 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` ## [Unreleased] ### Added +- Check every self-update before it lands: read each runtime file the fetched + commits add or change straight from git, scan it for leftover merge conflict + markers, and parse it if it's Python. If a file fails, refuse the update + before anything is stashed or merged, and record the failure on the + dashboard. Skip that commit quietly until the remote moves, with a reminder + once a day. Updates now fast-forward to exactly the commit that was checked. +- Run the pulse through a pre-flight (`bin/run-pulse.py`) that parses `pulse.py` + and the modules it loads at startup before each start. If one won't parse, + the pre-flight skips the run and exits cleanly instead of crashing. The + dashboard shows the error at the top of the page until a pulse runs again, and + an actions-ledger entry reaches Slack for a new error, then once a day. New installs + get this right away. Existing machines pick it up after a reboot, a logout, or + a manual reload of the pulse LaunchAgent, since self-update defers that reload. - Enforce 100% changed-code coverage with separate Python and real-browser reports. Add missing failure-path tests and repeatable mutation checks for key protections. Bind reports to measured sources and correctly map multiline Python and JavaScript changes. @@ -27,6 +40,7 @@ The version is carried in `pyproject.toml` and `src/assistant/__init__.py` and reminders to finish older pending work before starting another task. ### Fixed +- Pin the clock in the review-topic focus test so it stops failing once its fixture alert is more than 4 days old. - Download browser-check dependencies publicly so CI doesn't require Adobe's internal network. - Invalidate return notes after completed tool traffic; require review before reusing older notes. - Block close-out for unverified terminals, and safely show incomplete question choices. diff --git a/bin/pulse.py b/bin/pulse.py index 612eb21..2860de8 100755 --- a/bin/pulse.py +++ b/bin/pulse.py @@ -457,10 +457,12 @@ def self_update_pulse(pulse_idx: int) -> None: reason = result.get("skipped_reason") changed = result.get("changed") - # Silent path: attempted, nothing to do, no problem. - if not changed and reason is None and not result.get("error"): + # Silent path: attempted, nothing to do, no problem — or a refused commit + # whose failure was recorded in the last day. + if not changed and reason in (None, "syntax-fail-known") and not result.get("error"): return + kind = "self-update" if changed: files = result.get("files_changed", []) installed = result.get("installed") @@ -495,6 +497,15 @@ def self_update_pulse(pulse_idx: int) -> None: outcome = "failed" evidence = f"self-update auto-stash failed: {result.get('error', '')}"[:300] key = f"self-update-stash-failed-p{pulse_idx}" + elif reason == "syntax-fail": + # A fetched file has conflict markers or Python that won't parse; + # self_update refused the commits before touching the working tree. + outcome = "failed" + kind = "self-update-syntax-fail" + evidence = (f"refused self-update {result.get('from_sha')}.." + f"{result.get('to_sha')} (pull by hand if this is wrong): " + f"{result.get('syntax_error', '')}")[:300] + key = f"self-update-syntax-fail-p{pulse_idx}" else: outcome = "failed" evidence = f"self-update {reason or 'error'}: {result.get('error', '')}"[:300] @@ -505,7 +516,7 @@ def self_update_pulse(pulse_idx: int) -> None: "epoch": utc_ts(), "pulse_idx": pulse_idx, "key": key, - "kind": "self-update", + "kind": kind, "ws_ref": "(launchd)", "outcome": outcome, "evidence": evidence, diff --git a/bin/render-assistant-page.py b/bin/render-assistant-page.py index 9b89114..e154dac 100755 --- a/bin/render-assistant-page.py +++ b/bin/render-assistant-page.py @@ -2254,6 +2254,27 @@ def _ws_num(c): return f'
{"".join(col_html)}
', total +def render_pulse_alert() -> str: + """A top-of-page alert when bin/run-pulse.py refused to start the pulse and + no pulse has run since. Empty string when there's nothing to show.""" + try: + record = json.loads((HOME / ".assistant/pulse-preflight.json").read_text()) + failed_at, error = float(record["failed_at"]), str(record["error"]) + except (OSError, ValueError, TypeError, KeyError): + return "" + try: + last_ts = float(json.loads((HOME / ".assistant/heartbeat.json").read_text()) + .get("last_pulse_ts") or 0) + except (OSError, ValueError, TypeError, AttributeError): + last_ts = 0 + if failed_at <= last_ts: + return "" + return ('') + + def render_pulse_health() -> str: """One-line banner showing whether the assistant-pulse cron is alive. Reads ~/.assistant/heartbeat.json and color-codes by age: @@ -2451,6 +2472,7 @@ def render(): brief_html, brief_n = render_brief_tab() connections_html, connected_n = render_connections_panel(world) pulse_health_html = render_pulse_health() + pulse_alert_html = render_pulse_alert() counts = world.get("counts", {}) snapshot_at = _overview_timestamp(world.get("_meta", {}).get("built_at")) rendered_at = utc_now().timestamp() @@ -4076,6 +4098,7 @@ def render(): Checks for updates every 15 seconds. +{pulse_alert_html}
Background services and saved data {pulse_health_html}

The page checks for updates every 15 seconds, except while you're reading expanded details.

diff --git a/bin/run-pulse.py b/bin/run-pulse.py new file mode 100755 index 0000000..ff05248 --- /dev/null +++ b/bin/run-pulse.py @@ -0,0 +1,81 @@ +#!/usr/bin/env python3 +"""Launchd pre-flight for the Assistant pulse. + +Parses the files the pulse loads at startup, clears any earlier failure record, +then replaces this process with bin/pulse.py. If one won't parse, it skips the +run and exits 0 instead of starting a pulse that would crash on import: + + - ~/.assistant/pulse-preflight.json holds the error; the dashboard shows it + at the top of the page until a pulse runs again. + - An actions-ledger entry (so Slack hears about it) is written for a new + error, then once a day while it lasts. + - stderr (the LaunchAgent's err log) gets one line per skipped run. + +The LaunchAgent fires every StartInterval whatever the exit code, so the next +tick retries. Modules the pulse loads later aren't checked here: it guards +those imports itself, and self_update.py refuses incoming commits that don't +parse. +""" +from __future__ import annotations + +import json +import os +import sys +import time +from datetime import datetime, timezone +from pathlib import Path + +BIN = Path(__file__).resolve().parent +PULSE = BIN / "pulse.py" +# pulse.py plus its unguarded module-level imports. Keep in sync with pulse.py. +STARTUP_FILES = (PULSE, BIN.parent / "src/assistant/__init__.py", + BIN.parent / "src/assistant/model_tiers.py") +LEDGER_EVERY_SEC = 86400 # 1 day + + +def _record_failure(assistant_dir: Path, error: str, now: float) -> None: + record = assistant_dir / "pulse-preflight.json" + try: + previous = json.loads(record.read_text()) + except (OSError, ValueError): + previous = {} + ledgered_at = previous.get("ledgered_at") if previous.get("error") == error else None + try: + assistant_dir.mkdir(parents=True, exist_ok=True) + if ledgered_at is None or now - ledgered_at >= LEDGER_EVERY_SEC: + stamp = datetime.fromtimestamp(now, timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + with open(assistant_dir / "actions-ledger.jsonl", "a") as ledger: + ledger.write(json.dumps({ + "ts": stamp, "epoch": int(now), "key": f"pulse-preflight-fail-{int(now)}", + "kind": "pulse-preflight-fail", "ws_ref": "(launchd)", "outcome": "failed", + "evidence": f"pulse can't start: {error}"[:300], + }) + "\n") + ledgered_at = now + tmp = record.with_suffix(".json.tmp") + tmp.write_text(json.dumps({"failed_at": now, "error": error, "ledgered_at": ledgered_at})) + tmp.replace(record) + except OSError as exc: + print(f"pulse pre-flight could not record the failure in {assistant_dir}: {exc}", + file=sys.stderr) + + +def main(argv: list[str], *, execv=os.execv) -> int: + assistant_dir = Path.home() / ".assistant" + for path in STARTUP_FILES: + try: + compile(path.read_bytes(), str(path), "exec", dont_inherit=True) + except (OSError, SyntaxError, ValueError) as exc: + error = f"{path}: {type(exc).__name__}: {exc}" + now = time.time() + _record_failure(assistant_dir, error, now) + stamp = datetime.fromtimestamp(now, timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + print(f"[{stamp}] pulse pre-flight FAILED, skipping this run: {error}", + file=sys.stderr) + return 0 + (assistant_dir / "pulse-preflight.json").unlink(missing_ok=True) + execv(sys.executable, [sys.executable, str(PULSE), *argv]) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/bin/self_update.py b/bin/self_update.py index 89ac10a..5e68346 100644 --- a/bin/self_update.py +++ b/bin/self_update.py @@ -19,8 +19,14 @@ clocked from the first pulse that observed it dirty) AND an update is waiting, in which case the tree is auto-stashed (`git stash push -u`, always recoverable via `git stash pop`) and the pull proceeds. - 3. If behind: `git pull --ff-only `. Fast-forward only, - so a diverged history fails loudly rather than merging blindly. + Before any stash or pull, every runtime file the fetched commits add or + change is read straight from git: all are scanned for conflict markers, + and Python files are parsed. A broken commit is refused and never reaches + the working tree; it's skipped quietly until the remote moves, with a + reminder once a day. + 3. If behind: `git merge --ff-only ` — exactly the commit the + gate checked. Fast-forward only, so a diverged history fails loudly + rather than merging blindly. bin/ and prompts/ are symlinked / read live, so a pull alone makes code + Observer-prompt changes take effect on the very next pulse. 4. If the pull touched COPIED artifacts (skills/, launchagents/, the @@ -37,6 +43,7 @@ import json import subprocess +import sys import time from pathlib import Path @@ -61,6 +68,18 @@ # (`git stash list` / `git stash pop`); it is never dropped or discarded. DEFAULT_DIRTY_STASH_AFTER_SEC = 86400 # 1 day +# ── Pre-pull syntax gate ───────────────────────────────────────────────────── +# Paths the running system loads, runs, or installs from the checkout. Add new +# runtime paths here. In July 2026, conflict markers left in the working copy of +# bin/pulse.py made the pulse fail every tick for months. The gate keeps a +# pulled commit from doing the same; bin/run-pulse.py covers the working copy. +SYNTAX_GATE_PATHS = ("bin/", "src/", "hooks/", "install/", "prompts/", "skills/", + "launchagents/", "config/", "docs/", "slack-reactor/", + "install.sh", "install-bootstrap.sh") + +# A refused commit is re-checked, and its failure re-recorded, this often. +REJECT_REMIND_SEC = 86400 # 1 day + def _git(repo: Path, *args: str, timeout: int = 90) -> tuple[int, str, str]: """Run a git command in `repo`. Returns (rc, stdout, stderr); never raises.""" @@ -178,6 +197,64 @@ def should_attempt(marker: dict, now: float, interval_sec: int) -> bool: return (now - last) >= interval_sec +def _conflict_marker_line(text: str) -> int | None: + """Line number of the first git conflict marker in `text`, else None. + + Only column-0 markers count. A bare `=======` line alone is a legitimate + RST section underline, so it counts only when the text also carries a + `<<<<<<<` or `>>>>>>>` line.""" + first = None + arrow = False + for n, line in enumerate(text.splitlines(), 1): + if line.startswith(("<<<<<<<", ">>>>>>>")): + arrow = True + elif not line.startswith("======="): + continue + first = first or n + return first if arrow else None + + +def _blob(repo: Path, sha: str, name: str) -> bytes | None: + """Raw bytes of `name` at commit `sha`, or None if git can't read it.""" + try: + p = subprocess.run(["git", "-C", str(repo), "cat-file", "blob", f"{sha}:{name}"], + capture_output=True, timeout=90) + except subprocess.TimeoutExpired: + return None + return p.stdout if p.returncode == 0 else None + + +def syntax_gate(repo: Path, old_sha: str, new_sha: str) -> tuple[str, str]: + """Check the files under SYNTAX_GATE_PATHS that `new_sha` adds or changes + relative to `old_sha`: none may carry conflict markers, and Python files + must parse. Reads committed blobs, never the working tree. + + Returns (verdict, detail). verdict is "ok", "broken" (a file has conflict + markers or won't parse), or "unchecked" (git couldn't list or read the + files). detail names the first problem, or "ok".""" + rc, raw, err = _git(repo, "diff", "--raw", "--no-renames", "-z", "--diff-filter=ACMT", + old_sha, new_sha, "--", *SYNTAX_GATE_PATHS) + if rc != 0: + return "unchecked", f"could not list incoming changes: {err}"[:500] + fields = raw.split("\0") + for meta, name in zip(fields[::2], fields[1::2]): + if not meta.split()[1].startswith("100"): + continue # a symlink or submodule has no file content to check + source = _blob(repo, new_sha, name) + if source is None: + return "unchecked", f"could not read {name} at {new_sha[:12]}" + marker = _conflict_marker_line(source.decode("utf-8", errors="replace")) + if marker: + return "broken", f"conflict marker at {name}:{marker}" + if not name.endswith(".py"): + continue + try: + compile(source, name, "exec", dont_inherit=True) + except (SyntaxError, ValueError) as exc: + return "broken", f"{name}: {exc}"[:500] + return "ok", "ok" + + def maybe_update( repo: Path, *, @@ -259,14 +336,42 @@ def _log(msg: str) -> None: _log("already up to date") return result + if status["dirty"] and result["dirty_age_sec"] < dirty_stash_after_sec: + age = result["dirty_age_sec"] + result["skipped_reason"] = "dirty" + _log(f"working tree dirty for {age / 3600.0:.1f}h " + f"(< {dirty_stash_after_sec / 3600.0:.0f}h) — refusing to pull " + "(surfacing instead)") + return result + + # Gate the fetched commit before stashing or pulling anything, so broken + # code never reaches the working tree. A commit this interpreter already + # refused is skipped quietly until the remote moves or a day passes. + old_head, new_sha = status["head"], status["remote_sha"] + result["to_sha"] = new_sha[:12] + rejected = f"{new_sha} python{sys.version_info[0]}.{sys.version_info[1]}" + if (marker.get("rejected") == rejected + and now - marker.get("rejected_ts", 0) < REJECT_REMIND_SEC): + result["skipped_reason"] = "syntax-fail-known" + _log(f"{remote}/{branch} is still at refused {new_sha[:12]}; waiting for a fix") + return result + verdict, detail = syntax_gate(repo, old_head, new_sha) + if verdict == "unchecked": + result["skipped_reason"] = "gate-error" + result["error"] = detail + _log(f"could not check {new_sha[:12]}; not updating this time: {detail[:200]}") + return result + if verdict == "broken": + marker["rejected"], marker["rejected_ts"] = rejected, now + _write_marker(marker_path, marker) + result["skipped_reason"] = "syntax-fail" + result["syntax_error"] = detail + _log(f"refused {old_head[:12]}..{new_sha[:12]}, a file failed the check: " + f"{detail[:200]}") + return result + if status["dirty"]: age = result["dirty_age_sec"] - if age < dirty_stash_after_sec: - result["skipped_reason"] = "dirty" - _log(f"working tree dirty for {age / 3600.0:.1f}h " - f"(< {dirty_stash_after_sec / 3600.0:.0f}h) — refusing to pull " - "(surfacing instead)") - return result # Dirty past the window AND an update is waiting → stash, then pull. # The stash is recoverable (`git stash list` / `git stash pop`); it is # never dropped. @@ -285,12 +390,11 @@ def _log(msg: str) -> None: "pull; recover with `git stash pop`") # Fast-forward only — a diverged history fails rather than merging blindly. - old_head = status["head"] - rc, _, err = _git(repo, "pull", "--ff-only", remote, branch) + rc, _, err = _git(repo, "merge", "--ff-only", new_sha) if rc != 0: result["skipped_reason"] = "pull-failed" result["error"] = err - _log(f"git pull --ff-only failed: {err}") + _log(f"git merge --ff-only {new_sha[:12]} failed: {err}") return result _, new_head, _ = _git(repo, "rev-parse", "HEAD") diff --git a/launchagents/com.assistant.assistant-pulse.plist b/launchagents/com.assistant.assistant-pulse.plist index 769cd96..a0f37b0 100644 --- a/launchagents/com.assistant.assistant-pulse.plist +++ b/launchagents/com.assistant.assistant-pulse.plist @@ -7,7 +7,7 @@ ProgramArguments __PYTHON__ - __REPO__/bin/pulse.py + __REPO__/bin/run-pulse.py StartInterval 300 diff --git a/src/assistant/subsystems/pulse.py b/src/assistant/subsystems/pulse.py index 7a8c9df..8e81252 100644 --- a/src/assistant/subsystems/pulse.py +++ b/src/assistant/subsystems/pulse.py @@ -17,8 +17,9 @@ - The pulse spawns its own Observer subprocesses, writes its own heartbeat, and is the most safety-critical component. A subprocess gives us complete isolation and byte-for-byte compatibility with the system that runs today: - the daemon runs EXACTLY `python3 bin/pulse.py`, the same command the - com.assistant.assistant-pulse LaunchAgent runs. + the daemon runs EXACTLY `python3 bin/pulse.py`, the command the + com.assistant.assistant-pulse LaunchAgent execs after its bin/run-pulse.py + pre-flight. So this subsystem is a clean supervisor loop: run one pulse, sleep `pulse_interval_sec`, repeat — interruptible on shutdown. Bedrock env is merged diff --git a/tests/test_pulse_topup.py b/tests/test_pulse_topup.py index 8ee4f69..ef9c8ca 100644 --- a/tests/test_pulse_topup.py +++ b/tests/test_pulse_topup.py @@ -216,6 +216,47 @@ def test_self_update_reason_stash_failed(mod, home): assert e["key"] == "self-update-stash-failed-p9" +def test_self_update_reason_syntax_fail(mod, home): + _inject_self_update(mod, { + "changed": False, "skipped_reason": "syntax-fail", + "from_sha": "aaaaaaaaaaaa", "to_sha": "bbbbbbbbbbbb", + "syntax_error": "conflict marker at bin/pulse.py:42", + }) + try: + mod.self_update_pulse(13) + finally: + sys.modules.pop("self_update", None) + e = _read_ledger(home)[0] + assert e["outcome"] == "failed" + assert e["kind"] == "self-update-syntax-fail" + assert e["key"] == "self-update-syntax-fail-p13" + assert e["evidence"] == ("refused self-update aaaaaaaaaaaa..bbbbbbbbbbbb " + "(pull by hand if this is wrong): " + "conflict marker at bin/pulse.py:42") + + +def test_self_update_already_refused_commit_is_silent(mod, home): + _inject_self_update(mod, {"changed": False, "skipped_reason": "syntax-fail-known"}) + try: + mod.self_update_pulse(14) + finally: + sys.modules.pop("self_update", None) + assert _read_ledger(home) == [] + + +def test_self_update_gate_error_is_recorded(mod, home): + _inject_self_update(mod, {"changed": False, "skipped_reason": "gate-error", + "error": "could not read bin/pulse.py at bbbb"}) + try: + mod.self_update_pulse(15) + finally: + sys.modules.pop("self_update", None) + e = _read_ledger(home)[0] + assert e["outcome"] == "failed" + assert e["kind"] == "self-update" + assert e["evidence"] == "self-update gate-error: could not read bin/pulse.py at bbbb" + + def test_self_update_other_reason_failed(mod, home): _inject_self_update(mod, { "changed": False, "skipped_reason": "pull-failed", diff --git a/tests/test_renderer_brief_tab.py b/tests/test_renderer_brief_tab.py index 510a7b2..862bd08 100644 --- a/tests/test_renderer_brief_tab.py +++ b/tests/test_renderer_brief_tab.py @@ -317,6 +317,10 @@ def test_alert_created_after_focus_unpins_topic_without_hiding_history(self): self.write_brief(brief_fixture()) checked_at = "2026-09-19T10:10:00-07:00" checked_epoch = datetime.fromisoformat(checked_at).timestamp() + # Pin the clock an hour after the check: freshness decays to 0 within + # 4 days, after which "New topic alert" no longer outranks "Old topic alert". + self.enterContext(patch.object(self.mod.brief_store.time, "time", + return_value=checked_epoch + 3600)) old = {"id": "old-alert", "title": "Old topic alert", "source": "github", "refs": {"repo": "adobe/firefly-platform", "pr": 15561}, "created_epoch": NOW, "epoch": checked_epoch + 60} diff --git a/tests/test_renderer_in_process.py b/tests/test_renderer_in_process.py index ecc6512..6db3f79 100644 --- a/tests/test_renderer_in_process.py +++ b/tests/test_renderer_in_process.py @@ -104,6 +104,40 @@ def test_stale_pulse_renders_red_banner(self): self.assertIn("pulse-bad", html) self.assertIn("Pulse stale", html) + def _write_preflight(self, payload) -> None: + (self._tmp / ".assistant/pulse-preflight.json").write_text( + payload if isinstance(payload, str) else json.dumps(payload)) + + def test_preflight_failure_after_last_pulse_renders_top_alert(self): + now = int(time.time()) + self._write_heartbeat({"last_pulse_ts": now - 30, "pulse_idx": 99, "model": "m"}) + self._write_preflight({"failed_at": now - 10, + "error": "bin/pulse.py: SyntaxError: (line 1)"}) + self.assertEqual(self.mod.render_pulse_alert(), ( + '")) + self.assertIn("Pulse healthy", self.mod.render_pulse_health()) + + def test_preflight_failure_with_no_heartbeat_renders_top_alert(self): + self._write_preflight({"failed_at": 5, "error": "first run broke"}) + self.assertIn("Pulse can't start: first run broke", self.mod.render_pulse_alert()) + + def test_preflight_failure_before_last_pulse_is_ignored(self): + now = int(time.time()) + self._write_heartbeat({"last_pulse_ts": now - 30, "pulse_idx": 99, "model": "m"}) + self._write_preflight({"failed_at": now - 60, "error": "old failure"}) + self.assertEqual(self.mod.render_pulse_alert(), "") + + def test_unreadable_preflight_record_is_ignored(self): + for payload in ("{ corrupt", {"error": "no timestamp"}): + with self.subTest(payload=payload): + self._write_preflight(payload) + self.assertEqual(self.mod.render_pulse_alert(), "") + + def test_no_preflight_record_renders_nothing(self): + self.assertEqual(self.mod.render_pulse_alert(), "") + def test_age_formatting_includes_unit(self): now = int(time.time()) with mock.patch.object(self.mod, "utc_now", diff --git a/tests/test_run_pulse_wrapper.py b/tests/test_run_pulse_wrapper.py new file mode 100644 index 0000000..7df10a1 --- /dev/null +++ b/tests/test_run_pulse_wrapper.py @@ -0,0 +1,204 @@ +"""Tests for bin/run-pulse.py — the launchd pre-flight for the pulse. + +In-process tests call the real module with a fake `execv` so the parse check, +the logged skip, and the exact exec command are measured. Subprocess tests copy +the real file next to a stub pulse and let it exec for real. +""" +from __future__ import annotations + +import ast +import importlib.util +import json +import os +import plistlib +import runpy +import shutil +import subprocess +import sys +from pathlib import Path +from unittest import mock + +import pytest + +REPO = Path(__file__).resolve().parent.parent +WRAPPER = REPO / "bin/run-pulse.py" + + +def _load(): + spec = importlib.util.spec_from_file_location("run_pulse_mod", str(WRAPPER)) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def _layout(tmp: Path, pulse_body: str, model_tiers: str = "TIERS = {}\n") -> Path: + (tmp / "bin").mkdir() + shutil.copy2(WRAPPER, tmp / "bin/run-pulse.py") + (tmp / "bin/pulse.py").write_text(pulse_body) + (tmp / "src/assistant").mkdir(parents=True) + (tmp / "src/assistant/__init__.py").write_text("") + (tmp / "src/assistant/model_tiers.py").write_text(model_tiers) + return tmp / "bin/run-pulse.py" + + +def test_healthy_checkout_clears_old_failure_and_execs_pulse(tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + record = tmp_path / ".assistant/pulse-preflight.json" + record.parent.mkdir() + record.write_text('{"failed_at": 1, "error": "old"}') + mod = _load() + calls = [] + assert mod.main(["--pulse-idx", "5"], execv=lambda *a: calls.append(a)) == 0 + assert calls == [(sys.executable, [sys.executable, str(REPO / "bin/pulse.py"), + "--pulse-idx", "5"])] + assert not record.exists() + + +def test_broken_pulse_skips_the_run_and_records_why(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path / "home")) + _layout(tmp_path, "def broken(:\n pass\n") + mod = _load() + calls = [] + with mock.patch.object(mod, "STARTUP_FILES", (tmp_path / "bin/pulse.py",)): + assert mod.main([], execv=lambda *a: calls.append(a)) == 0 + assert calls == [] + err = capsys.readouterr().err + assert "pulse pre-flight FAILED, skipping this run" in err + assert f"{tmp_path / 'bin/pulse.py'}: SyntaxError" in err + record = json.loads((tmp_path / "home/.assistant/pulse-preflight.json").read_text()) + assert record["error"].startswith(f"{tmp_path / 'bin/pulse.py'}: SyntaxError") + assert record["ledgered_at"] == record["failed_at"] + [entry] = [json.loads(line) for line in + (tmp_path / "home/.assistant/actions-ledger.jsonl").read_text().splitlines()] + assert entry["kind"] == "pulse-preflight-fail" + assert entry["outcome"] == "failed" + assert entry["evidence"] == f"pulse can't start: {record['error']}"[:300] + + +def test_ledger_entry_is_written_for_a_new_error_then_once_a_day(tmp_path): + mod = _load() + ledger = tmp_path / "actions-ledger.jsonl" + + def entries(): + return [json.loads(line)["evidence"] for line in ledger.read_text().splitlines()] + + mod._record_failure(tmp_path, "err A", 1000.0) + mod._record_failure(tmp_path, "err A", 1000.0 + 3600) + assert entries() == ["pulse can't start: err A"] + mod._record_failure(tmp_path, "err B", 1000.0 + 7200) + mod._record_failure(tmp_path, "err B", 1000.0 + 7200 + 86400) + assert entries() == ["pulse can't start: err A", "pulse can't start: err B", + "pulse can't start: err B"] + record = json.loads((tmp_path / "pulse-preflight.json").read_text()) + assert record == {"failed_at": 1000.0 + 7200 + 86400, "error": "err B", + "ledgered_at": 1000.0 + 7200 + 86400} + + +def test_unwritable_record_still_skips_cleanly(tmp_path, capsys, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path / "home")) + (tmp_path / "home").mkdir() + (tmp_path / "home/.assistant").write_text("a file where the folder should be") + _layout(tmp_path, "def broken(:\n") + mod = _load() + calls = [] + with mock.patch.object(mod, "STARTUP_FILES", (tmp_path / "bin/pulse.py",)): + assert mod.main([], execv=lambda *a: calls.append(a)) == 0 + assert calls == [] + err = capsys.readouterr().err + assert "could not record the failure" in err + assert "pulse pre-flight FAILED, skipping this run" in err + + +def test_broken_startup_import_skips_the_run(tmp_path): + wrapper = _layout(tmp_path, 'print("RAN")\n', model_tiers="x = 1\n<<<<<<< HEAD\n") + r = subprocess.run([sys.executable, str(wrapper)], capture_output=True, text=True, + timeout=60, env=dict(os.environ, HOME=str(tmp_path / "home"))) + assert r.returncode == 0 + assert r.stdout == "" + assert "src/assistant/model_tiers.py: SyntaxError" in r.stderr + + +def test_missing_startup_file_skips_the_run(tmp_path): + wrapper = _layout(tmp_path, 'print("RAN")\n') + (tmp_path / "src/assistant/model_tiers.py").rename(tmp_path / "src/assistant/moved.py") + r = subprocess.run([sys.executable, str(wrapper)], capture_output=True, text=True, + timeout=60, env=dict(os.environ, HOME=str(tmp_path / "home"))) + assert r.returncode == 0 + assert r.stdout == "" + assert "model_tiers.py: FileNotFoundError" in r.stderr + + +def test_broken_later_module_does_not_block_the_pulse(tmp_path): + # The pulse guards the modules it loads later; an unrelated broken module + # must not stop every run. + wrapper = _layout(tmp_path, 'print("RAN")\n') + (tmp_path / "src/assistant/narrator.py").write_text("def broken(:\n") + r = subprocess.run([sys.executable, str(wrapper)], capture_output=True, text=True, + timeout=60, env=dict(os.environ, HOME=str(tmp_path / "home"))) + assert r.returncode == 0 + assert r.stdout.strip() == "RAN" + + +def _module_level_imports(nodes): + for node in nodes: + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + continue + if isinstance(node, (ast.Import, ast.ImportFrom)): + yield node + for field in ("body", "orelse", "finalbody", "handlers"): + yield from _module_level_imports(getattr(node, field, [])) + + +def test_startup_files_match_pulse_module_level_imports(): + tree = ast.parse((REPO / "bin/pulse.py").read_text()) + imported = {"bin/pulse.py"} + for node in _module_level_imports(tree.body): + modules = ([alias.name for alias in node.names] if isinstance(node, ast.Import) + else [node.module]) + for module in modules: + top = module.split(".")[0] + if top in sys.stdlib_module_names or top == "__future__": + continue + assert module == "assistant", f"add {module} to run-pulse.py STARTUP_FILES" + imported.add("src/assistant/__init__.py") + imported.update(f"src/assistant/{alias.name}.py" for alias in node.names) + listed = {str(path.relative_to(REPO)) for path in _load().STARTUP_FILES} + assert imported == listed + + +def test_script_entry_point_execs_pulse(tmp_path, monkeypatch): + monkeypatch.setenv("HOME", str(tmp_path)) + calls = [] + with mock.patch.object(os, "execv", lambda *a: calls.append(a)), \ + mock.patch.object(sys, "argv", [str(WRAPPER), "--dry-run"]): + with pytest.raises(SystemExit) as exc: + runpy.run_path(str(WRAPPER), run_name="__main__") + assert exc.value.code == 0 + assert calls == [(sys.executable, [sys.executable, str(REPO / "bin/pulse.py"), + "--dry-run"])] + + +def test_real_exec_runs_pulse_and_passes_args(tmp_path): + wrapper = _layout(tmp_path, 'import sys\nprint("RAN", " ".join(sys.argv[1:]))\n') + r = subprocess.run([sys.executable, str(wrapper), "--pulse-idx", "5"], + capture_output=True, text=True, timeout=60, + env=dict(os.environ, HOME=str(tmp_path / "home"))) + assert r.returncode == 0 + assert r.stdout.strip() == "RAN --pulse-idx 5" + assert r.stderr == "" + + +def test_real_run_with_broken_pulse_exits_zero(tmp_path): + wrapper = _layout(tmp_path, "def broken(:\n") + r = subprocess.run([sys.executable, str(wrapper)], + capture_output=True, text=True, timeout=60, + env=dict(os.environ, HOME=str(tmp_path / "home"))) + assert r.returncode == 0 + assert r.stdout == "" + assert "pulse pre-flight FAILED" in r.stderr + + +def test_pulse_launch_agent_runs_the_preflight(): + plist = plistlib.loads((REPO / "launchagents/com.assistant.assistant-pulse.plist") + .read_bytes()) + assert plist["ProgramArguments"] == ["__PYTHON__", "__REPO__/bin/run-pulse.py"] diff --git a/tests/test_self_update.py b/tests/test_self_update.py index dce96c9..110943e 100644 --- a/tests/test_self_update.py +++ b/tests/test_self_update.py @@ -12,6 +12,7 @@ import json import os import subprocess +import sys import unittest import unittest.mock from pathlib import Path @@ -480,6 +481,217 @@ def test_git_os_error_returns_minus1(self): self.assertIn("no git binary", err) +class SyntaxGateTests(unittest.TestCase): + """The pre-pull gate: fetched commits whose Python won't parse (conflict + markers or a SyntaxError) are refused before anything is stashed, merged, + or reset — so the pulse can never crash on them the way it did in July 2026.""" + + def _bad_remote(self, tmp: Path, files: dict[str, str]) -> tuple[Path, str, str]: + clone, remote = make_repos(tmp) + old_head = git(clone, "rev-parse", "HEAD") + new_sha = advance_remote(tmp, remote, files, "incoming") + return clone, old_head, new_sha + + def _gate(self, files: dict[str, str]) -> tuple[str, str]: + with TemporaryDirectory() as t: + clone, old_head, new_sha = self._bad_remote(Path(t), files) + git(clone, "fetch", "origin", "main") + return su.syntax_gate(clone, old_head, new_sha) + + def test_conflict_markers_are_refused_before_touching_the_tree(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, old_head, new_sha = self._bad_remote( + tmp, {"bin/pulse.py": "# pulse\n<<<<<<< HEAD\na = 1\n=======\na = 2\n>>>>>>> x\n"}) + reflog = git(clone, "reflog") + r = su.maybe_update(clone, interval_sec=0, marker_path=tmp / "m.json") + self.assertEqual(r["skipped_reason"], "syntax-fail") + self.assertFalse(r["changed"]) + self.assertEqual(r["syntax_error"], "conflict marker at bin/pulse.py:2") + self.assertEqual(r["to_sha"], new_sha[:12]) + self.assertEqual(git(clone, "rev-parse", "HEAD"), old_head) + self.assertEqual(git(clone, "reflog"), reflog) # no merge, no reset + self.assertEqual((clone / "bin/pulse.py").read_text(), "# pulse\n") + self.assertEqual(json.loads((tmp / "m.json").read_text())["rejected"], + f"{new_sha} python{sys.version_info[0]}.{sys.version_info[1]}") + + def test_broken_runtime_python_outside_bin_is_refused(self): + for rel in ("src/assistant/model_tiers.py", "hooks/cmux-session-ledger.py", + "install/patch-settings.py"): + with self.subTest(rel=rel), TemporaryDirectory() as t: + tmp = Path(t) + clone, old_head, _ = self._bad_remote(tmp, {rel: "def broken(:\n"}) + r = su.maybe_update(clone, interval_sec=0, marker_path=tmp / "m.json") + self.assertEqual(r["skipped_reason"], "syntax-fail") + self.assertTrue(r["syntax_error"].startswith(f"{rel}: ")) + self.assertEqual(git(clone, "rev-parse", "HEAD"), old_head) + self.assertFalse((clone / rel).exists()) + + def test_conflict_markers_in_runtime_non_python_files_are_refused(self): + verdict, detail = self._gate({"install.sh": "<<<<<<< HEAD\necho a\n=======\necho b\n"}) + self.assertEqual(verdict, "broken") + self.assertEqual(detail, "conflict marker at install.sh:1") + + def test_refused_commit_is_skipped_until_the_remote_moves(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, remote = make_repos(tmp) + advance_remote(tmp, remote, {"bin/pulse.py": "def broken(:\n"}, "bad") + marker = tmp / "m.json" + self.assertEqual(su.maybe_update(clone, interval_sec=0, marker_path=marker) + ["skipped_reason"], "syntax-fail") + with unittest.mock.patch.object(su, "syntax_gate") as gate: + r = su.maybe_update(clone, interval_sec=0, marker_path=marker) + gate.assert_not_called() + self.assertEqual(r["skipped_reason"], "syntax-fail-known") + self.assertEqual(r["to_sha"], git(clone, "rev-parse", "origin/main")[:12]) + fixed = advance_remote(tmp, remote, {"bin/pulse.py": "fixed = True\n"}, "fix") + r = su.maybe_update(clone, interval_sec=0, marker_path=marker) + self.assertTrue(r["changed"]) + self.assertEqual(git(clone, "rev-parse", "HEAD"), fixed) + self.assertEqual((clone / "bin/pulse.py").read_text(), "fixed = True\n") + + def test_refused_commit_is_recorded_again_after_a_day(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, _, _ = self._bad_remote(tmp, {"bin/pulse.py": "def broken(:\n"}) + marker = tmp / "m.json" + runs = [su.maybe_update(clone, interval_sec=0, marker_path=marker, now=when) + ["skipped_reason"] for when in (1000.0, 1000.0 + 3600, 1000.0 + 86400)] + self.assertEqual(runs, ["syntax-fail", "syntax-fail-known", "syntax-fail"]) + self.assertEqual(json.loads(marker.read_text())["rejected_ts"], 1000.0 + 86400) + + def test_refusal_by_another_python_is_checked_again(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, _, new_sha = self._bad_remote(tmp, {"bin/pulse.py": "def broken(:\n"}) + marker = tmp / "m.json" + su.maybe_update(clone, interval_sec=0, marker_path=marker, now=1000.0) + saved = json.loads(marker.read_text()) + marker.write_text(json.dumps({**saved, "rejected": f"{new_sha} python3.0"})) + r = su.maybe_update(clone, interval_sec=0, marker_path=marker, now=1001.0) + self.assertEqual(r["skipped_reason"], "syntax-fail") + + def test_git_trouble_during_the_check_is_not_remembered(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, _, new_sha = self._bad_remote(tmp, {"bin/pulse.py": "ok = 1\n"}) + marker = tmp / "m.json" + with unittest.mock.patch.object(su, "syntax_gate", + return_value=("unchecked", "git timed out")): + r = su.maybe_update(clone, interval_sec=0, marker_path=marker) + self.assertEqual(r["skipped_reason"], "gate-error") + self.assertEqual(r["error"], "git timed out") + self.assertNotIn("rejected", json.loads(marker.read_text())) + r = su.maybe_update(clone, interval_sec=0, marker_path=marker) + self.assertTrue(r["changed"]) + self.assertEqual(git(clone, "rev-parse", "HEAD"), new_sha) + + def test_dirty_tree_past_window_is_not_stashed_for_a_refused_update(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, _, _ = self._bad_remote(tmp, {"bin/pulse.py": "def broken(:\n"}) + (clone / "install.sh").write_text("# operator edit\n") + marker = tmp / "m.json" + first = su.maybe_update(clone, interval_sec=0, marker_path=marker, + dirty_stash_after_sec=86400, now=1000.0) + later = su.maybe_update(clone, interval_sec=0, marker_path=marker, + dirty_stash_after_sec=86400, now=1000.0 + 25 * 3600) + self.assertEqual(first["skipped_reason"], "dirty") + self.assertEqual(later["skipped_reason"], "syntax-fail") + self.assertNotIn("stashed", later) + self.assertEqual(git(clone, "stash", "list"), "") + self.assertEqual((clone / "install.sh").read_text(), "# operator edit\n") + + def test_healthy_update_lands_on_the_gated_commit(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, _, new_sha = self._bad_remote(tmp, { + "bin/pulse.py": '"""Pulse.\n\nSection\n=======\n"""\nok = True\n', + "tests/test_scratch.py": "def broken(:\n", + "README.md": "<<<<<<< HEAD\n=======\n>>>>>>> x\n", + "prompts/observer.md": "Heading\n=======\n", + "skills/logo.bin": "\x00\x01binary", + }) + r = su.maybe_update(clone, interval_sec=0, marker_path=tmp / "m.json") + self.assertTrue(r["changed"]) + self.assertIsNone(r["skipped_reason"]) + self.assertEqual(git(clone, "rev-parse", "HEAD"), new_sha) + + def test_symlinked_python_file_is_not_compiled_as_its_link_text(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, remote = make_repos(tmp) + old_head = git(clone, "rev-parse", "HEAD") + scratch = tmp / "scratch-link" + git(tmp, "clone", str(remote), str(scratch)) + git(scratch, "config", "user.email", "t@t") + git(scratch, "config", "user.name", "t") + (scratch / "bin/alias.py").symlink_to("../bin/pulse.py") + git(scratch, "add", "-A") + git(scratch, "commit", "-m", "add symlink") + git(scratch, "push", "origin", "main") + git(clone, "fetch", "origin", "main") + verdict, detail = su.syntax_gate(clone, old_head, git(scratch, "rev-parse", "HEAD")) + self.assertEqual((verdict, detail), ("ok", "ok")) + + def test_marker_inside_a_string_is_flagged_even_though_it_compiles(self): + verdict, detail = self._gate({"bin/pulse.py": 'X = """\n>>>>>>> theirs\n"""\n'}) + self.assertEqual(verdict, "broken") + self.assertEqual(detail, "conflict marker at bin/pulse.py:2") + + def test_marker_line_ignores_rst_underline_alone(self): + self.assertIsNone(su._conflict_marker_line("Title\n=======\nbody\n")) + self.assertEqual(su._conflict_marker_line("a\n=======\n>>>>>>> b\n"), 2) + + def test_unreadable_diff_is_refused(self): + with TemporaryDirectory() as t: + clone, _ = make_repos(Path(t)) + verdict, detail = su.syntax_gate(clone, "0" * 40, "HEAD") + self.assertEqual(verdict, "unchecked") + self.assertTrue(detail.startswith("could not list incoming changes: ")) + + def test_unreadable_blob_is_refused(self): + with TemporaryDirectory() as t: + clone, old_head, new_sha = self._bad_remote(Path(t), {"bin/new.py": "x = 1\n"}) + git(clone, "fetch", "origin", "main") + real_run = subprocess.run + + def run(cmd, **kwargs): + if "cat-file" in cmd: + raise subprocess.TimeoutExpired(cmd, 90) + return real_run(cmd, **kwargs) + + with unittest.mock.patch.object(su.subprocess, "run", run): + verdict, detail = su.syntax_gate(clone, old_head, new_sha) + self.assertEqual(verdict, "unchecked") + self.assertEqual(detail, f"could not read bin/new.py at {new_sha[:12]}") + + def test_blob_missing_from_commit_reads_as_none(self): + with TemporaryDirectory() as t: + clone, _ = make_repos(Path(t)) + self.assertIsNone(su._blob(clone, "HEAD", "bin/absent.py")) + self.assertEqual(su._blob(clone, "HEAD", "bin/pulse.py"), b"# pulse\n") + + def test_failed_fast_forward_is_reported(self): + with TemporaryDirectory() as t: + tmp = Path(t) + clone, old_head, new_sha = self._bad_remote(tmp, {"bin/pulse.py": "ok = 1\n"}) + real_git = su._git + + def fake_git(repo, *args, **kwargs): + if args[0] == "merge": + self.assertEqual(args, ("merge", "--ff-only", new_sha)) + return 1, "", "not possible to fast-forward" + return real_git(repo, *args, **kwargs) + + with unittest.mock.patch.object(su, "_git", fake_git): + r = su.maybe_update(clone, interval_sec=0, marker_path=tmp / "m.json") + self.assertEqual(r["skipped_reason"], "pull-failed") + self.assertEqual(r["error"], "not possible to fast-forward") + self.assertEqual(git(clone, "rev-parse", "HEAD"), old_head) + + class ResolveRemoteBranchTests(unittest.TestCase): def test_detached_head_returns_none(self): """When HEAD is detached, resolve_remote_branch returns None.""" diff --git a/tests/test_self_update_topup.py b/tests/test_self_update_topup.py index aa391a6..ee08e6c 100644 --- a/tests/test_self_update_topup.py +++ b/tests/test_self_update_topup.py @@ -90,7 +90,8 @@ def test_maybe_update_stash_failed(tmp_path): "head": "oldsha", "remote_sha": "newsha", "dirty": True, "behind": 1, "ahead": 0}): with mock.patch.object(su, "_stash_dirty", - return_value=(False, "fatal: stash conflict")): + return_value=(False, "fatal: stash conflict")), \ + mock.patch.object(su, "syntax_gate", return_value=("ok", "ok")): # First pass stamps dirty_since at t=1000. su.maybe_update(tmp_path, interval_sec=0, marker_path=marker, dirty_stash_after_sec=86400, now=1000.0) @@ -107,12 +108,12 @@ def test_maybe_update_stash_failed(tmp_path): # ─── maybe_update: pull-failed path (291-294) ───────────────────────────────── def test_maybe_update_pull_failed(tmp_path): - """Clean tree, behind, but `git pull --ff-only` fails (e.g. diverged) → + """Clean tree, behind, but `git merge --ff-only` fails (e.g. diverged) → skipped_reason 'pull-failed' with the git error captured.""" marker = tmp_path / "m.json" def fake_git(repo, *args, **kw): - if args[:1] == ("pull",): + if args[:1] == ("merge",): return (1, "", "fatal: Not possible to fast-forward, aborting.") # rev-parse HEAD after a failed pull would not be reached. return (0, "", "") @@ -137,7 +138,7 @@ def test_maybe_update_pull_noop_to_sha_set(tmp_path): marker = tmp_path / "m.json" def fake_git(repo, *args, **kw): - if args[:1] == ("pull",): + if args[:1] == ("merge",): return (0, "Already up to date.", "") if args[:2] == ("rev-parse", "HEAD"): return (0, "oldsha", "") # same as status head → no change