From 7f8c8e867dc55e8af624c09588d2f6252e93e84b Mon Sep 17 00:00:00 2001 From: musingfox Date: Tue, 10 Nov 2026 13:36:23 +0800 Subject: [PATCH 1/3] fix(trial): a dry run checks the service token file The dry run skipped `.env.trial-.access`, which the real run refuses on when it is missing or holds no service token, so a dry run passed for a trial the real run would stop. It now reads the file first, before any deploy, so a dry run refuses what the real run refuses. Two tests pin it: a service token file that is empty or lacks the secret gives a dry run the same error the real run gives, and a missing file is refused with no deploy. Co-Authored-By: Claude Opus 5.5 --- scripts/deploy_trial.py | 5 +++-- tests/test_deploy_trial.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/scripts/deploy_trial.py b/scripts/deploy_trial.py index d7763726..e7fd6e17 100644 --- a/scripts/deploy_trial.py +++ b/scripts/deploy_trial.py @@ -27,7 +27,8 @@ deploy.yml's: a warm instance keeps answering from the image it started with. --dry-run resolves the image, renders the config and runs `wrangler deploy ---dry-run` for each Worker it would deploy; it deploys nothing and saves nothing. +--dry-run` for each Worker it would deploy; it deploys nothing and saves nothing. It reads +`.env.trial-.access` first, as the real run does, so it refuses what the real run refuses. Usage: deploy_trial.py --slug S [--rss] [--promote] [--app-only] [--dry-run] [--wait SECONDS] @@ -309,7 +310,7 @@ def main( step = Deploy(args, run or _runner(wrangler_env(args.slug)), say) commit = step.commit() # Read before anything deploys, so a deploy that cannot be checked never starts. - headers = {} if args.dry_run else step.access_headers() + headers = step.access_headers() digest = step.digest(commit) say(f"{image_tag(commit)}: {commit} -> {digest}") state = provision_trial.wizard_state(args.slug) diff --git a/tests/test_deploy_trial.py b/tests/test_deploy_trial.py index 2753e92d..385c6f9f 100644 --- a/tests/test_deploy_trial.py +++ b/tests/test_deploy_trial.py @@ -329,6 +329,34 @@ def test_no_service_token_stops_before_any_deploy(deploy, tree: Path, capsys) -> assert runner.deploys() == [] +@pytest.mark.parametrize("access", ["", "CYRIS_ACCESS_CLIENT_ID=cid\n"], ids=["empty", "no-secret"]) +def test_a_dry_run_refuses_a_service_token_file_the_real_run_refuses( + deploy, tree: Path, capsys, access: str +) -> None: + (tree / f".env.trial-{SLUG}.access").write_text(access) + real, dry = FakeRunner(), FakeRunner() + + assert _main(deploy, real)[0] == 1 + real_err = capsys.readouterr().err + code, *_ = _main(deploy, dry, "--dry-run") + + assert code == 1 + assert capsys.readouterr().err == real_err + assert ".access holds no CYRIS_ACCESS_CLIENT_" in real_err + assert dry.deploys() == [] + + +def test_a_dry_run_with_no_service_token_file_is_refused(deploy, tree: Path, capsys) -> None: + (tree / f".env.trial-{SLUG}.access").unlink() + runner = FakeRunner() + + code, *_ = _main(deploy, runner, "--dry-run") + + assert code == 1 + assert f".env.trial-{SLUG}.access holds no" in capsys.readouterr().err + assert runner.deploys() == [] + + def test_the_build_is_asked_through_access_with_no_login(deploy) -> None: seen: list[httpx.Request] = [] From f8cefd8c1c3dcd4482fbfb6ce423ff34dd2a9c9e Mon Sep 17 00:00:00 2001 From: musingfox Date: Tue, 10 Nov 2026 13:37:44 +0800 Subject: [PATCH 2/3] feat(trial): deploy_trial.py runs cyris doctor after the build matches On 2026-10-21 trial1 had not published since 10-15: its D1 lacked two required settings keys, the run container exited in require_complete_settings and wrote no digest_runs row, and the deploy only compared /api/build. After the build comparison passes, the script runs `cyris doctor` once from a temporary directory holding an empty cyris.toml and a .env built from .env.trial- with CYRIS_STORE_BACKEND=d1 (cyris reads only the .env beside its config). The process starts from `env -i`. The run role's other defaults, HTML output and publishing on, are added unless the file sets them, so doctor judges what the scheduled run starts from. A failed check exits non-zero and prints the failed lines, which name the missing keys. A missing .env.trial- stops the script before anything deploys, and so does one whose database id is not the trial's: - Doctor applies the schema to the database the file names, and nothing compared that id with TRIAL_D1_ID. A mismatch, or no id, stops before any deploy and names both ids. - The id is read as cyris reads it. cyris loads the file with os.environ.setdefault: the first line wins, stripped. provision_trial.env_file takes the last line, unstripped, so a file with another deployment's id early and the trial's later passed the check while doctor applied the schema to the other database, and a trailing space after the right id was refused. A file holding the key on more than one line is refused, naming the lines; a single line is compared stripped. A dry run reads the file and makes the same check instead of skipping doctor's inputs; it does not run doctor. Tests pin what a review's mutations left green: the run role's defaults never override the trial's own file value, the temporary .env is chmod 0600 whatever the umask, the temporary directory is removed after a passing and a failing run, and the failure shows the failed checks and their fixes and no passing line. Co-Authored-By: Claude Opus 5.5 --- docs/architecture.md | 9 +- docs/operations.md | 3 +- scripts/deploy_trial.py | 110 ++++++++- tests/test_deploy_trial.py | 24 +- tests/test_deploy_trial_doctor.py | 367 ++++++++++++++++++++++++++++++ 5 files changed, 506 insertions(+), 7 deletions(-) create mode 100644 tests/test_deploy_trial_doctor.py diff --git a/docs/architecture.md b/docs/architecture.md index 8ff5290a..bce458df 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -643,7 +643,14 @@ ids and progress in `.env.trial--wizard` at the repo root, beside the tria those secrets files are the only copy, since Cloudflare cannot read a secret back. Only `scripts/deploy_trial.py` changes that file after the wizard: it records the image it staged there, so a later wizard run renders that image and not the first one, and the commit each of the -rss and vote Workers was last deployed from, so the next staging knows whether to redeploy them. A +rss and vote Workers was last deployed from, so the next staging knows whether to redeploy them. +`deploy_trial.py` runs `cyris doctor` on the trial's D1 settings once `/api/build` names the deployed commit, because a D1 +missing a required key serves that commit while every run exits at once. It reads `.env.trial-` once, before any +deploy and in a `--dry-run` as in a real run, to build the `.env` doctor runs on; a dry run also reads +`.env.trial-.access` as a real run does, and refuses a missing service token. Doctor applies the schema to the +database that file names, so the script first checks that it names the state's `TRIAL_D1_ID`, reading the value as +cyris does (stripped, the first line winning), and stops before any deploy when it does not, or when the file holds +`CYRIS_STORE_DATABASE_ID` on more than one line. A trial's reader content can also come from production: `scripts/seed_trial.py save` copies production's rows of the tables `scripts/reset_trial.py` clears into `~/cyris-trial-seeds/`, outside the repo, and `restore` writes them into a trial's D1, never its `settings` or diff --git a/docs/operations.md b/docs/operations.md index 42b607f5..4add0fe2 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -94,7 +94,8 @@ before it. This project's trial is `trial1`. 1. Merge to `main`. The release runs, and its deploy job deploys production. 2. From the checkout that provisioned the trial, with that commit checked out, run `scripts/deploy_trial.py`. It deploys the image the release recorded, and fails - unless the trial reports that commit. To check onboarding from its first step, + unless the trial reports that commit and `cyris doctor`, run locally on the trial's + `.env.trial-` against its D1, reports no error. To check onboarding from its first step, clear the trial with `scripts/reset_trial.py` first. To check it from real content, restore a seed of production's with `scripts/seed_trial.py` first. Each script describes itself under `--help`. diff --git a/scripts/deploy_trial.py b/scripts/deploy_trial.py index e7fd6e17..eb134e47 100644 --- a/scripts/deploy_trial.py +++ b/scripts/deploy_trial.py @@ -26,20 +26,29 @@ this commit. The wait is deploy.yml's: a warm instance keeps answering from the image it started with. +Then it runs `cyris doctor` once against the trial's settings, locally, from +`.env.trial-` with `CYRIS_STORE_BACKEND=d1`, and fails naming what doctor +reports as an error. /api/build only says which image answers: a D1 missing a +required settings key still serves it while every scheduled run exits at once. + --dry-run resolves the image, renders the config and runs `wrangler deploy ---dry-run` for each Worker it would deploy; it deploys nothing and saves nothing. It reads -`.env.trial-.access` first, as the real run does, so it refuses what the real run refuses. +--dry-run` for each Worker it would deploy; it deploys nothing and saves nothing. It makes +the same checks of `.env.trial-` and `.env.trial-.access` as the real run, before +any deploy. Usage: deploy_trial.py --slug S [--rss] [--promote] [--app-only] [--dry-run] [--wait SECONDS] """ import argparse +import os import re import subprocess import sys +import tempfile import time from collections.abc import Callable +from pathlib import Path import httpx import provision_trial @@ -58,6 +67,14 @@ STATE_KEYS = ("DOMAIN", "ACCOUNT_ID", "TRIAL_KV_ID", "TRIAL_D1_ID") BASELINE_KEYS = {"rss": "RSS_WORKER_COMMIT", "promote": "PROMOTE_WORKER_COMMIT"} UNKNOWN = "unknown" +DOCTOR_DB_KEY = "CYRIS_STORE_DATABASE_ID" +DOCTOR_FAIL = "✗" +# What docker/entrypoint.sh's `run` role exports unless the environment already has it, so +# doctor judges the settings the scheduled run starts from. The backend is forced, never kept. +RUN_ROLE_DEFAULTS = { + "CYRIS_HTML_OUTPUT_ENABLED": "true", + "CYRIS_PROMOTE_PUBLISH_ENABLED": "true", +} class DeployError(Exception): @@ -103,6 +120,20 @@ def workers_to_deploy( return out +def doctor_failures(report: str) -> list[str]: + """The failed checks in `cyris doctor`'s report, each with the indented fix under it.""" + out: list[str] = [] + failing = False + for line in report.splitlines(): + if line.startswith(DOCTOR_FAIL): + failing = True + elif not line.startswith(" "): + failing = False + if failing: + out.append(line) + return out + + def write_state(slug: str, values: dict[str, str]) -> None: """Replace or append KEY=value lines, as the wizard's write_env does.""" path = ROOT / f".env.trial-{slug}-wizard" @@ -238,6 +269,74 @@ def access_headers(self) -> dict[str, str]: raise DeployError(f"{name} holds no {' or '.join(missing)} to sign in with") return {header: values[var] for var, header in ACCESS_HEADERS.items()} + def doctor_env(self, state: dict[str, str]) -> str: + """The trial's `.env`, as the run container reads it, on D1; read before any deploy. + + Doctor applies the schema to the database that file names, so that database must be + the one the wizard state holds for this trial. The id is read as cyris reads it, the + value stripped, which is why a file naming it twice is refused: cyris would take the + first line, `provision_trial.env_file` the last. A dry run reads it too. The file is read + once, here. + """ + name = f".env.trial-{self.slug}" + path = ROOT / name + if not path.exists(): + raise DeployError(f"{name} not found: doctor needs it to reach the trial's D1") + lines = path.read_text(encoding="utf-8").splitlines() + held_at = [ + (number, line.partition("=")[2].strip()) + for number, line in enumerate(lines, 1) + if (stripped := line.strip()) + and not stripped.startswith("#") + and "=" in stripped + and stripped.partition("=")[0].strip() == DOCTOR_DB_KEY + ] + if len(held_at) > 1: + numbers = ", ".join(str(number) for number, _ in held_at) + raise DeployError( + f"{name} has {DOCTOR_DB_KEY} on lines {numbers}: cyris takes the first, the" + " scripts here the last, so doctor could apply the schema to a database that is" + " not this trial's. Keep one line" + ) + named = held_at[0][1] if held_at else "" + if not named or named != state.get("TRIAL_D1_ID"): + raise DeployError( + f"{name} has {DOCTOR_DB_KEY}={named or '(none)'}, but .env.trial-{self.slug}-wizard" + f" has TRIAL_D1_ID={state.get('TRIAL_D1_ID') or '(none)'}: cyris doctor applies the" + " schema to the database it names, so it is not run on one that is not this trial's" + ) + held = {line.split("=", 1)[0].strip() for line in lines} + kept = [line for line in lines if line.split("=", 1)[0].strip() != "CYRIS_STORE_BACKEND"] + defaults = [f"{k}={v}" for k, v in RUN_ROLE_DEFAULTS.items() if k not in held] + return "\n".join([*kept, *defaults, "CYRIS_STORE_BACKEND=d1"]) + "\n" + + def doctor(self, dotenv: str) -> None: + """Run `cyris doctor` once on the trial's settings; an error in its report fails the deploy. + + cyris reads only the `.env` beside its config file, so both go in one temporary + directory. The process starts from an empty environment, as the wizard's own + `settings push` does, so nothing the shell exported can stand in for the trial's. + """ + with tempfile.TemporaryDirectory() as tmp: + config, sources = Path(tmp) / "cyris.toml", Path(tmp) / "sources.yaml" + config.write_text("", encoding="utf-8") + sources.write_text("sources: []\n", encoding="utf-8") + (Path(tmp) / ".env").write_text(dotenv, encoding="utf-8") + (Path(tmp) / ".env").chmod(0o600) + keep = [f"{k}={os.environ[k]}" for k in ("HOME", "PATH") if k in os.environ] + cmd = ["env", "-i", *keep, "uv", "run", "cyris", "doctor"] + cmd += ["--config", str(config), "--sources", str(sources)] + self.say("$ " + " ".join(cmd)) + done = self._run(cmd) + if done.returncode == 0: + self.say("cyris doctor: ready to run") + return + failed = doctor_failures(done.stdout) or [done.stderr.strip() or done.stdout.strip()] + raise DeployError( + f"cyris doctor reports an error on {self.slug}, though the build matches:\n" + + "\n".join(failed) + ) + def verify( self, commit: str, @@ -310,10 +409,11 @@ def main( step = Deploy(args, run or _runner(wrangler_env(args.slug)), say) commit = step.commit() # Read before anything deploys, so a deploy that cannot be checked never starts. + state = provision_trial.wizard_state(args.slug) headers = step.access_headers() + dotenv = step.doctor_env(state) digest = step.digest(commit) say(f"{image_tag(commit)}: {commit} -> {digest}") - state = provision_trial.wizard_state(args.slug) rendered = _snapshot(args.slug) app_deployed = False try: @@ -352,7 +452,9 @@ def main( return 0 host = configs["app"]["routes"][0]["pattern"] step.verify(commit, f"https://{host}", headers, fetch, sleep) - say(f"https://{host} serves {commit}: the trial runs this release.") + say(f"https://{host} serves {commit}.") + step.doctor(dotenv) + say("The trial runs this release and its settings are complete.") return 0 except (DeployError, provision_trial.ProductionNameError, provision_trial.DriftError) as exc: print(str(exc), file=sys.stderr) diff --git a/tests/test_deploy_trial.py b/tests/test_deploy_trial.py index 385c6f9f..f50ffb73 100644 --- a/tests/test_deploy_trial.py +++ b/tests/test_deploy_trial.py @@ -10,7 +10,7 @@ import httpx import pytest -from trial_tree import DIGEST, SLUG, load, make_tree +from trial_tree import D1, DIGEST, SLUG, load, make_tree pytestmark = pytest.mark.unit @@ -62,6 +62,7 @@ def tree(tmp_path: Path) -> Path: (t / f".env.trial-{SLUG}.access").write_text( "CYRIS_ACCESS_CLIENT_ID=cid\nCYRIS_ACCESS_CLIENT_SECRET=sek-SECRET\n", encoding="utf-8" ) + (t / f".env.trial-{SLUG}").write_text(f"CYRIS_STORE_DATABASE_ID={D1}\n", encoding="utf-8") return t @@ -116,6 +117,27 @@ def test_an_optional_worker_deploys_when_changed_unknown_or_asked(deploy) -> Non assert pick({"rss": True, "promote": None}, set(), True) == {} +REPORT = """\ +✓ config file — read +✗ settings — missing x + push x with `cyris settings push` +! llm — slow + retry later +✗ pages — unreachable +✓ sources — 3 + +2 problem(s) would break a run.""" + + +def test_doctor_failures_keeps_each_failed_check_with_its_fix_and_nothing_else(deploy) -> None: + assert deploy.doctor_failures(REPORT) == [ + "✗ settings — missing x", + " push x with `cyris settings push`", + "✗ pages — unreachable", + ] + assert deploy.doctor_failures("✓ a — fine\n\nReady to run.") == [] + + # --- the run diff --git a/tests/test_deploy_trial_doctor.py b/tests/test_deploy_trial_doctor.py new file mode 100644 index 00000000..b9a85891 --- /dev/null +++ b/tests/test_deploy_trial_doctor.py @@ -0,0 +1,367 @@ +"""scripts/deploy_trial.py runs `cyris doctor` on the trial's settings once the build matches. + +The 2026-10-21 failure: trial1's D1 lacked two required settings keys, every run exited +in `require_complete_settings`, and /api/build still named the deployed commit. Here the +fake runner answers git, bun and wrangler as test_deploy_trial.py does, and answers the +`cyris doctor` command by running the real CLI against a fake D1 behind the one seam, +`build_d1_client`, holding whichever settings the test stored. The Pages probe is the other +fake: doctor would otherwise call Cloudflare. +""" + +import os +import stat +from pathlib import Path + +import pytest +from fakes import SqliteD1, seed_d1_settings +from test_deploy_trial import COMMIT, OTHER, FakeRunner +from trial_tree import D1, SLUG, load, make_tree +from typer.testing import CliRunner + +from cyris.config import REQUIRED_KEYS +from cyris.entrypoints.cli import app + +pytestmark = pytest.mark.integration + +TOKEN = "tok-trial-SECRET" +ACCOUNT = "0123456789abcdef0123456789abcdef" +DOTENV = ( + f"CLOUDFLARE_ACCOUNT_ID={ACCOUNT}\nCLOUDFLARE_API_TOKEN={TOKEN}\nCYRIS_STORE_DATABASE_ID={D1}\n" + "CYRIS_PROMOTE_PAGES_PROJECT=cyris-alice-0123456789abcdef\n" +) + + +class DoctorRunner(FakeRunner): + """FakeRunner whose `uv run cyris doctor` is the real command over `d1`.""" + + def __init__(self, d1: SqliteD1, monkeypatch, **kwargs) -> None: + super().__init__(**kwargs) + self.d1 = d1 + self.monkeypatch = monkeypatch + self.reached: list[tuple[str, str]] = [] + self.dotenv = "" + self.dotenv_mode = 0 + self.workdir: Path | None = None + + def doctors(self) -> list[list[str]]: + return [c for c in self.calls if c[-4:-3] == ["doctor"] or "doctor" in c[:12]] + + def __call__(self, cmd: list[str]): + if "doctor" not in cmd: + return super().__call__(cmd) + import subprocess + + self.calls.append(cmd) + config = Path(cmd[cmd.index("--config") + 1]) + self.dotenv = (config.parent / ".env").read_text(encoding="utf-8") + self.dotenv_mode = stat.S_IMODE((config.parent / ".env").stat().st_mode) + self.workdir = config.parent + scrubbed = dict(arg.split("=", 1) for arg in cmd[2 : cmd.index("uv")] if "=" in arg) + + def client(cfg): + self.reached.append((cfg.app.store.database_id, cfg.app.store.api_token)) + return self.d1 + + with self.monkeypatch.context() as m: + m.setattr(os, "environ", scrubbed) + m.setattr("cyris.bootstrap.build_d1_client", client) + m.setattr( + "cyris.adapters.cloudflare.check_pages_access", + lambda account, project, token: (True, f"{project} reachable"), + ) + result = CliRunner().invoke( + app, ["doctor", *cmd[cmd.index("--config") :]], catch_exceptions=False + ) + return subprocess.CompletedProcess(cmd, result.exit_code, result.output, "") + + +@pytest.fixture +def tree(tmp_path: Path) -> Path: + t = make_tree(tmp_path, "deploy_trial") + (t / f".env.trial-{SLUG}.access").write_text( + "CYRIS_ACCESS_CLIENT_ID=cid\nCYRIS_ACCESS_CLIENT_SECRET=sek-SECRET\n", encoding="utf-8" + ) + (t / f".env.trial-{SLUG}").write_text(DOTENV, encoding="utf-8") + return t + + +@pytest.fixture +def deploy(tree: Path, monkeypatch): + return load(tree, "deploy_trial", monkeypatch) + + +def _d1(**seed) -> SqliteD1: + db = SqliteD1() + seed_d1_settings(db, **seed) + return db + + +def _main(deploy, runner, *args, online=COMMIT): + said: list[str] = [] + code = deploy.main( + ["--slug", SLUG, *args], + run=runner, + say=said.append, + fetch=lambda base, headers: online, + sleep=lambda _: None, + ) + return code, said + + +def test_complete_settings_let_the_deploy_succeed(deploy, monkeypatch, capsys) -> None: + runner = DoctorRunner(_d1(), monkeypatch) + + code, said = _main(deploy, runner) + + assert code == 0, capsys.readouterr().err + assert len(runner.doctors()) == 1 + assert "settings are complete" in said[-1] + + +def test_a_d1_missing_a_required_key_fails_the_deploy_and_names_it( + deploy, monkeypatch, capsys +) -> None: + lacking = list(REQUIRED_KEYS[:2]) + runner = DoctorRunner(_d1(omit=lacking), monkeypatch) + + code, said = _main(deploy, runner) + + err = capsys.readouterr().err + assert code == 1 + for key in lacking: + assert key in err + assert "cyris doctor reports an error" in err + assert "settings serves" not in "".join(said) + + +def test_doctor_reads_the_trials_own_env_beside_its_config_on_d1(deploy, monkeypatch, tree) -> None: + (tree / f".env.trial-{SLUG}").write_text(DOTENV + "CYRIS_STORE_BACKEND=sqlite\n") + runner = DoctorRunner(_d1(), monkeypatch) + + code, _ = _main(deploy, runner) + + assert code == 0 + assert set(runner.reached) == {(D1, TOKEN)} + assert runner.dotenv.splitlines()[-1] == "CYRIS_STORE_BACKEND=d1" + assert "CYRIS_STORE_BACKEND=sqlite" not in runner.dotenv + + +def test_doctor_starts_from_an_empty_environment(deploy, monkeypatch) -> None: + runner = DoctorRunner(_d1(), monkeypatch) + monkeypatch.setenv("CLOUDFLARE_API_TOKEN", "from-the-shell") + + _main(deploy, runner) + + cmd = runner.doctors()[0] + assert cmd[:2] == ["env", "-i"] + assert not any("from-the-shell" in arg for arg in cmd) + assert set(runner.reached) == {(D1, TOKEN)} + + +def test_a_build_that_does_not_match_never_reaches_doctor(deploy, monkeypatch) -> None: + runner = DoctorRunner(_d1(omit=list(REQUIRED_KEYS[:1])), monkeypatch) + + code, _ = _main(deploy, runner, online=OTHER) + + assert code == 1 + assert runner.doctors() == [] + + +def test_a_dry_run_does_not_run_doctor(deploy, monkeypatch) -> None: + runner = DoctorRunner(_d1(omit=list(REQUIRED_KEYS[:1])), monkeypatch) + + code, _ = _main(deploy, runner, "--dry-run") + + assert code == 0 + assert runner.doctors() == [] + + +def test_a_missing_env_file_stops_before_any_deploy(deploy, monkeypatch, tree, capsys) -> None: + (tree / f".env.trial-{SLUG}").unlink() + runner = DoctorRunner(_d1(), monkeypatch) + + code, _ = _main(deploy, runner) + + assert code == 1 + assert f".env.trial-{SLUG} not found" in capsys.readouterr().err + assert runner.deploys() == [] + + +OTHER_D1 = "99999999-2222-4333-8444-555555555555" + + +def test_an_env_naming_another_database_stops_before_doctor_and_any_deploy( + deploy, monkeypatch, tree, capsys +) -> None: + (tree / f".env.trial-{SLUG}").write_text(DOTENV.replace(D1, OTHER_D1), encoding="utf-8") + runner = DoctorRunner(_d1(), monkeypatch) + + code, said = _main(deploy, runner) + + err = capsys.readouterr().err + assert code == 1 + assert OTHER_D1 in err and D1 in err + assert runner.doctors() == [] + assert runner.deploys() == [] + assert runner.reached == [] + + +def test_an_env_naming_no_database_stops_before_doctor(deploy, monkeypatch, tree, capsys) -> None: + dotenv = "".join( + ln + "\n" for ln in DOTENV.splitlines() if not ln.startswith("CYRIS_STORE_DATABASE_ID") + ) + (tree / f".env.trial-{SLUG}").write_text(dotenv, encoding="utf-8") + runner = DoctorRunner(_d1(), monkeypatch) + + code, _ = _main(deploy, runner) + + assert code == 1 + assert "CYRIS_STORE_DATABASE_ID=(none)" in capsys.readouterr().err + assert runner.doctors() == [] + + +def test_a_value_the_trials_file_sets_beats_the_run_role_default(deploy, monkeypatch, tree) -> None: + (tree / f".env.trial-{SLUG}").write_text(DOTENV + "CYRIS_PROMOTE_PUBLISH_ENABLED=false\n") + runner = DoctorRunner(_d1(), monkeypatch) + + code, _ = _main(deploy, runner) + + assert code == 0 + lines = runner.dotenv.splitlines() + assert [ln for ln in lines if ln.startswith("CYRIS_PROMOTE_PUBLISH_ENABLED=")] == [ + "CYRIS_PROMOTE_PUBLISH_ENABLED=false" + ] + assert [ln for ln in lines if ln.startswith("CYRIS_HTML_OUTPUT_ENABLED=")] == [ + "CYRIS_HTML_OUTPUT_ENABLED=true" + ] + + +def test_the_temporary_env_holding_the_token_is_private_whatever_the_umask( + deploy, monkeypatch +) -> None: + runner = DoctorRunner(_d1(), monkeypatch) + before = os.umask(0o022) + try: + code, _ = _main(deploy, runner) + finally: + os.umask(before) + + assert code == 0 + assert runner.dotenv_mode == 0o600 + + +def test_the_temporary_directory_is_gone_after_a_passing_and_a_failing_doctor( + deploy, monkeypatch +) -> None: + passing = DoctorRunner(_d1(), monkeypatch) + failing = DoctorRunner(_d1(omit=list(REQUIRED_KEYS[:1])), monkeypatch) + + assert _main(deploy, passing)[0] == 0 + assert _main(deploy, failing)[0] == 1 + + for runner in (passing, failing): + assert runner.workdir is not None and not runner.workdir.exists() + + +def test_the_failure_prints_the_failed_checks_and_not_the_passing_ones( + deploy, monkeypatch, capsys +) -> None: + lacking = list(REQUIRED_KEYS[:2]) + runner = DoctorRunner(_d1(omit=lacking), monkeypatch) + + _main(deploy, runner) + + err = capsys.readouterr().err + assert err.count("✗") >= 1 + assert "✓" not in err + assert "problem(s) would break a run" not in err + + +def _env_with(tree: Path, text: str) -> None: + (tree / f".env.trial-{SLUG}").write_text(text, encoding="utf-8") + + +@pytest.mark.parametrize( + "ids", + [(OTHER_D1, D1), (D1, OTHER_D1), (D1, D1)], + ids=["other-first", "trial-first", "same-twice"], +) +def test_an_env_naming_the_database_twice_is_refused_with_its_lines_and_never_reaches_one( + deploy, monkeypatch, tree, capsys, ids +) -> None: + first, last = ids + _env_with(tree, f"CYRIS_STORE_DATABASE_ID={first}\n" + DOTENV.replace(D1, last)) + runner = DoctorRunner(_d1(), monkeypatch) + + code, _ = _main(deploy, runner) + + err = capsys.readouterr().err + assert code == 1, runner.reached + assert "lines 1, 4" in err + assert runner.reached == [] + assert runner.doctors() == [] + assert runner.deploys() == [] + + +@pytest.mark.parametrize( + "line", + [f"CYRIS_STORE_DATABASE_ID={D1} ", f" CYRIS_STORE_DATABASE_ID = {D1}\t"], + ids=["trailing-space", "spaced-key-and-value"], +) +def test_the_database_id_is_compared_as_cyris_reads_it_stripped( + deploy, monkeypatch, tree, line +) -> None: + _env_with(tree, DOTENV.replace(f"CYRIS_STORE_DATABASE_ID={D1}", line)) + runner = DoctorRunner(_d1(), monkeypatch) + + code, _ = _main(deploy, runner) + + assert code == 0 + assert {db for db, _ in runner.reached} == {D1} + + +def test_a_commented_out_database_line_is_not_a_second_line(deploy, monkeypatch, tree) -> None: + _env_with(tree, f"# CYRIS_STORE_DATABASE_ID={OTHER_D1}\n" + DOTENV) + runner = DoctorRunner(_d1(), monkeypatch) + + code, _ = _main(deploy, runner) + + assert code == 0 + assert {db for db, _ in runner.reached} == {D1} + + +@pytest.mark.parametrize( + "dotenv", + [ + pytest.param(DOTENV.replace(D1, OTHER_D1), id="other-database"), + pytest.param(f"CYRIS_STORE_DATABASE_ID={OTHER_D1}\n" + DOTENV, id="named-twice"), + pytest.param(DOTENV.replace(f"CYRIS_STORE_DATABASE_ID={D1}\n", ""), id="none"), + ], +) +def test_a_dry_run_refuses_what_the_real_run_refuses( + deploy, monkeypatch, tree, capsys, dotenv +) -> None: + _env_with(tree, dotenv) + real = DoctorRunner(_d1(), monkeypatch) + dry = DoctorRunner(_d1(), monkeypatch) + + assert _main(deploy, real)[0] == 1 + real_err = capsys.readouterr().err + code, _ = _main(deploy, dry, "--dry-run") + + assert code == 1 + assert capsys.readouterr().err == real_err + assert dry.deploys() == [] + + +def test_a_dry_run_with_a_missing_env_file_is_refused_as_the_real_run_is( + deploy, monkeypatch, tree, capsys +) -> None: + (tree / f".env.trial-{SLUG}").unlink() + runner = DoctorRunner(_d1(), monkeypatch) + + code, _ = _main(deploy, runner, "--dry-run") + + assert code == 1 + assert f".env.trial-{SLUG} not found" in capsys.readouterr().err + assert runner.deploys() == [] From 12583ad881b03b5fef26f3a5875ecad3d7091a96 Mon Sep 17 00:00:00 2001 From: musingfox Date: Tue, 10 Nov 2026 13:38:33 +0800 Subject: [PATCH 3/3] feat(trial): every trial path knows the newsletter Worker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #102 let the wizard create and deploy a per-trial newsletter Worker, but only the wizard knew about it. The state's TRIAL_NEWSLETTER_KV_ID alone now says whether a trial has one, in provision, deploy and reset alike; a trial wired before it has no id and each script leaves the Worker out. - provision_trial: rendered_paths() lists the newsletter config once it exists, load_rendered checks its name, binding and KV id for production names and drift, and production_names() includes the template's KV id. With no saved id, rendered_paths and load_rendered leave a config on disk out unread, so a stray one cannot stop reset or deploy. The old rule read and validated it either way. - deploy_trial: a trial whose state holds the id renders the config again and deploys it like rss and promote (changed directory, --newsletter, skipped by --app-only), with its own baseline NEWSLETTER_WORKER_COMMIT. A trial without the id is rendered and deployed as before. _snapshot and _restore cover the config, removing one this run rendered when the deploy fails. A dry run of a trial whose config is missing used to render it, restore the tree (deleting it again) and then raise an uncaught KeyError looking it up for the Worker's dry run; the Workers' dry runs now run before the restore, on the configs this run rendered. - reset_trial: clears the NEWSLETTERS KV's issues, keyed `nl:` as the Worker writes them, after checking the config's id against the state. Id saved, config missing: refused before any call, so the reset no longer prints "Reset" over a KV it did not clear. Config present, no id (trial1 today): the rest of the reset runs, the KV is left alone, and one line says so and to save the id in the wizard state. Reading the config on disk instead of the saved id made reset and deploy disagree both ways. - reset_trial and deploy_trial print the same one line for a stray config (provision_trial.newsletter_note) and do nothing to the file; deploy no longer snapshots or restores it. - The reset's refusal for an id with no config names the render command trial-wizard.sh stage 8 runs, built from the state's values, with a full deploy as the alternative. Each command sits on a line of its own with no trailing punctuation, so the d1 id does not read "." and the fallback does not print `--from 8.`. When DOMAIN, ACCOUNT_ID or IMAGE_DIGEST is missing the refusal names each missing state key and the wizard stage that saves it (2, 4 or 5) rather than `--from 8`: stage 8 reads those under set -u and a --from 8 run does not run stage 2, which writes them. - trial-wizard stage 14: a probe that gets no HTTP answer, or a 404, now says whether the deploy was refused, the URL could not be read from the deploy output, or the printed URL is not live, instead of "answered 000, not 401" or "answered 404, not 401", which blamed the Worker's answer for a Worker that was never deployed. A 404 from a URL this run's deploy printed keeps the plain wrong-answer message. An unreadable log no longer overwrites the saved URL. docs/architecture.md §4 states the contract for all three scripts, and that the newsletter Worker shares the rss and app bearer on purpose: WorkerConfig reads one CYRIS_WORKER_TOKEN for both server-to-server Workers, and ADR-0008 records that holding them apart bought rotation nobody used. operations.md and trial-deployment.md name the Worker too. Tests pin the contract: the stray-config report is read-free (Path reads, writes and mtime show deploy and its dry run never open, write or touch the config), and the note is absent when the id is saved. Co-Authored-By: Claude Opus 5.5 --- docs/architecture.md | 23 ++- docs/operations.md | 2 +- docs/trial-deployment.md | 3 +- scripts/deploy_trial.py | 114 ++++++++++--- scripts/provision_trial.py | 67 ++++++-- scripts/reset_trial.py | 134 ++++++++++++--- scripts/trial-wizard.sh | 55 ++++-- tests/test_deploy_trial.py | 307 +++++++++++++++++++++++++++++++++- tests/test_provision_trial.py | 57 +++++++ tests/test_reset_trial.py | 269 ++++++++++++++++++++++++++++- tests/test_trial_wizard.py | 70 +++++++- tests/trial_tree.py | 12 +- 12 files changed, 1027 insertions(+), 86 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index bce458df..7988c920 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -643,7 +643,20 @@ ids and progress in `.env.trial--wizard` at the repo root, beside the tria those secrets files are the only copy, since Cloudflare cannot read a secret back. Only `scripts/deploy_trial.py` changes that file after the wizard: it records the image it staged there, so a later wizard run renders that image and not the first one, and the commit each of the -rss and vote Workers was last deployed from, so the next staging knows whether to redeploy them. +rss, vote and newsletter Workers was last deployed from, so the next staging knows whether to redeploy +them. A trial whose state holds `TRIAL_NEWSLETTER_KV_ID` has that third Worker: `render` writes its +`workers/newsletter/wrangler.trial-.toml`, `load_rendered` checks it for production names and drift +as it does the other three, `deploy_trial.py` renders it again and deploys it, and `reset_trial.py` clears +its `NEWSLETTERS` KV. The state's id alone says whether a trial has that Worker, in all three scripts. A trial wired +before it has no id, and each script leaves the Worker out. A config on disk with no id is a state to report, not to +act on: `load_rendered` neither reads nor validates it, so a stray one cannot stop either script, and `reset_trial.py` and +`deploy_trial.py` each print the same one line saying that the config and its KV are left alone and that the id belongs +in the wizard state; the reset finishes the rest of its work and the deploy deploys the other Workers. An id with no +config on disk is refused by `reset_trial.py` before any call, naming the `provision_trial.py render` command built from +the state's values (the one the wizard's stage 8 runs) and a full deploy as the alternative, each on a line of its +own, or, when the state lacks a value that command needs, naming each missing key and the wizard stage that saves it, +and rendered again by +`deploy_trial.py`. A config whose KV id differs from the state's stops `reset_trial.py` before any KV delete. `deploy_trial.py` runs `cyris doctor` on the trial's D1 settings once `/api/build` names the deployed commit, because a D1 missing a required key serves that commit while every run exits at once. It reads `.env.trial-` once, before any deploy and in a `--dry-run` as in a real run, to build the `.env` doctor runs on; a dry run also reads @@ -656,6 +669,14 @@ production's rows of the tables `scripts/reset_trial.py` clears into `~/cyris-tr outside the repo, and `restore` writes them into a trial's D1, never its `settings` or `pages_*` rows, which name production's notification targets and Pages project. +The trial's newsletter Worker takes the same bearer as its rss Worker and its app, `TRIAL_WORKER_TOKEN`, +which the app presents as `CYRIS_WORKER_TOKEN`. That is deliberate and follows production: `WorkerConfig` +reads one `CYRIS_WORKER_TOKEN` for both server-to-server Workers, so a second value would have nowhere to +go in the app, and [ADR-0008](decisions/0008-worker-bearers-split-by-published-versus-secret.md) records +that the two were held apart before and it "bought independent rotation of keys nobody rotates", since the +same `.env` and the same Worker secret store hold both. The vote Worker's bearer stays separate, because +that one was once published. + ### One store, one truth `D1ArticleStore` satisfies `ArticleRepository`, and `[store] backend` picks where its tables are: diff --git a/docs/operations.md b/docs/operations.md index 4add0fe2..1fb7b186 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -102,7 +102,7 @@ before it. This project's trial is `trial1`. 3. Check the release on the trial. If it is wrong, take production back as *Going back* describes. -The rss and vote Workers deploy outside any workflow. `deploy_trial.py` deploys the +The rss, vote and newsletter Workers deploy outside any workflow. `deploy_trial.py` deploys the trial's copies when their code changed, and production's copies then need the same deploy by hand. diff --git a/docs/trial-deployment.md b/docs/trial-deployment.md index f0f6a08d..37bb0265 100644 --- a/docs/trial-deployment.md +++ b/docs/trial-deployment.md @@ -148,7 +148,8 @@ The wizard writes these at the repo root, all gitignored: A trial started before the newsletter Worker existed re-runs from stage 5, with `--from 5`, to create its newsletter KV namespace. Without it, stages 8, 9, 14 and 15 skip the -newsletter Worker and print that command. Its stage 13 pushes the settings alone, and +newsletter Worker and print that command, and `deploy_trial.py` and `reset_trial.py` leave it +out too. Its stage 13 pushes the settings alone, and `cyris settings push` never overwrites a row, so the tester's sources and `/settings` edits stay as they are. `trial1` was wired by hand before then and is left as it is, with no migration path. diff --git a/scripts/deploy_trial.py b/scripts/deploy_trial.py index eb134e47..8cc9ba7e 100644 --- a/scripts/deploy_trial.py +++ b/scripts/deploy_trial.py @@ -14,10 +14,13 @@ (`.env.trial--wizard`), whose IMAGE_TAG and IMAGE_DIGEST are then updated, so a later wizard run does not render the old image back. -The rss and vote Workers deploy too when their directory changed since the commit -each was last deployed from, or when --rss or --promote asks; --app-only skips -both. That commit is kept per Worker in the wizard state (RSS_WORKER_COMMIT, -PROMOTE_WORKER_COMMIT) and moves only when that Worker deploys, so a skipped or +The rss, vote and newsletter Workers deploy too when their directory changed since the +commit each was last deployed from, or when --rss, --promote or --newsletter asks; +--app-only skips all three. The newsletter Worker belongs only to a trial whose wizard +state holds TRIAL_NEWSLETTER_KV_ID: its config is rendered again with that id, and a +trial without one is rendered and deployed as before, with no newsletter Worker. That +commit is kept per Worker in the wizard state (RSS_WORKER_COMMIT, PROMOTE_WORKER_COMMIT, +NEWSLETTER_WORKER_COMMIT) and moves only when that Worker deploys, so a skipped or failed Worker deploy is offered again next time; before the first run it is the commit IMAGE_TAG names, the one the wizard provisioned with. @@ -36,8 +39,12 @@ the same checks of `.env.trial-` and `.env.trial-.access` as the real run, before any deploy. +A newsletter config on disk that the wizard state holds no TRIAL_NEWSLETTER_KV_ID for is +reported in one line, never read or validated, and left as it is. + Usage: - deploy_trial.py --slug S [--rss] [--promote] [--app-only] [--dry-run] [--wait SECONDS] + deploy_trial.py --slug S [--rss] [--promote] [--newsletter] [--app-only] [--dry-run] + [--wait SECONDS] """ import argparse @@ -63,9 +70,21 @@ "CYRIS_ACCESS_CLIENT_ID": "CF-Access-Client-Id", "CYRIS_ACCESS_CLIENT_SECRET": "CF-Access-Client-Secret", } -WORKER_DIRS = {"rss": "workers/rss", "promote": "workers/promote"} +WORKER_DIRS = { + "rss": "workers/rss", + "promote": "workers/promote", + "newsletter": "workers/newsletter", +} +# A trial wired before the newsletter Worker has no KV id for it, and so gets no such Worker. +NEWSLETTER_KV_KEY = provision_trial.NEWSLETTER_KV_KEY +# The Workers with a bun lockfile of their own to install before a deploy. +INSTALLED_WORKERS = ("rss", "newsletter") STATE_KEYS = ("DOMAIN", "ACCOUNT_ID", "TRIAL_KV_ID", "TRIAL_D1_ID") -BASELINE_KEYS = {"rss": "RSS_WORKER_COMMIT", "promote": "PROMOTE_WORKER_COMMIT"} +BASELINE_KEYS = { + "rss": "RSS_WORKER_COMMIT", + "promote": "PROMOTE_WORKER_COMMIT", + "newsletter": "NEWSLETTER_WORKER_COMMIT", +} UNKNOWN = "unknown" DOCTOR_DB_KEY = "CYRIS_STORE_DATABASE_ID" DOCTOR_FAIL = "✗" @@ -98,6 +117,11 @@ def recorded_digest(message: str, commit: str) -> str | None: return found.group(1) if found else None +def trial_workers(state: dict[str, str]) -> tuple[str, ...]: + """The optional Workers this trial has: the newsletter one only with its KV id saved.""" + return tuple(w for w in WORKER_DIRS if w != "newsletter" or state.get(NEWSLETTER_KV_KEY)) + + def build_matches(online: str, commit: str) -> bool: """Whether /api/build's git_sha names the deployed commit. An empty sha names none.""" return bool(online) and online == commit @@ -195,7 +219,8 @@ def baselines(self, state: dict[str, str]) -> dict[str, str | None]: it moves with every app deploy, which a skipped or failed Worker deploy does not. """ out: dict[str, str | None] = {} - for worker, key in BASELINE_KEYS.items(): + for worker in trial_workers(state): + key = BASELINE_KEYS[worker] if key in state: out[worker] = None if state[key] == UNKNOWN else state[key] elif tag := state.get("IMAGE_TAG"): @@ -207,8 +232,8 @@ def baselines(self, state: dict[str, str]) -> dict[str, str | None]: def changed(self, baselines: dict[str, str | None], commit: str) -> dict[str, bool | None]: out: dict[str, bool | None] = {} - for worker, path in WORKER_DIRS.items(): - before = baselines[worker] + for worker, before in baselines.items(): + path = WORKER_DIRS[worker] if not before: out[worker] = None continue @@ -222,6 +247,9 @@ def render(self, state: dict[str, str], digest: str) -> dict[str, dict]: raise DeployError( f".env.trial-{self.slug}-wizard lacks {', '.join(missing)}: run the wizard first" ) + newsletter = ( + ["--newsletter-kv-id", state[NEWSLETTER_KV_KEY]] if state.get(NEWSLETTER_KV_KEY) else [] + ) code = provision_trial.main( [ "render", @@ -235,6 +263,7 @@ def render(self, state: dict[str, str], digest: str) -> dict[str, dict]: digest, "--kv-id", state["TRIAL_KV_ID"], + *newsletter, "--d1-id", state["TRIAL_D1_ID"], ] @@ -261,6 +290,15 @@ def deploy(self, kind: str) -> None: self.say("$ " + " ".join(cmd)) self.sh(cmd) + def deploy_workers(self, workers: dict[str, str], commit: str | None = None) -> None: + """Install and deploy each Worker; with a `commit`, record it as that Worker's baseline.""" + for worker in workers: + if worker in INSTALLED_WORKERS: + self.sh(["bun", "install", "--frozen-lockfile", "--cwd", WORKER_DIRS[worker]]) + self.deploy(worker) + if commit: + write_state(self.slug, {BASELINE_KEYS[worker]: commit}) + def access_headers(self) -> dict[str, str]: name = f".env.trial-{self.slug}.access" values = provision_trial.env_file(name) @@ -361,14 +399,26 @@ def verify( raise DeployError(f"{base} does not report the deployed commit {commit}") -def _snapshot(slug: str) -> dict[str, bytes]: - paths = provision_trial.rendered_paths(slug).values() - return {rel: (ROOT / rel).read_bytes() for rel in paths} +def _snapshot(slug: str) -> dict[str, bytes | None]: + """Every config a render may write; None for the newsletter's when it is not rendered yet. + + A newsletter config the state holds no KV id for is not one a render writes, so it is not here. + """ + paths = list(provision_trial.rendered_paths(slug).values()) + if provision_trial.has_newsletter(slug): + paths.append(provision_trial.newsletter_path(slug)) + return { + rel: (ROOT / rel).read_bytes() if (ROOT / rel).exists() else None + for rel in dict.fromkeys(paths) + } -def _restore(snapshot: dict[str, bytes]) -> None: +def _restore(snapshot: dict[str, bytes | None]) -> None: for rel, data in snapshot.items(): - (ROOT / rel).write_bytes(data) + if data is None: + (ROOT / rel).unlink(missing_ok=True) + else: + (ROOT / rel).write_bytes(data) def _runner(env: dict[str, str]) -> Runner: @@ -387,6 +437,9 @@ def build_parser() -> argparse.ArgumentParser: parser.add_argument("--slug", required=True, help="the trial's slug, e.g. trial1") parser.add_argument("--rss", action="store_true", help="deploy the rss Worker too") parser.add_argument("--promote", action="store_true", help="deploy the vote Worker too") + parser.add_argument( + "--newsletter", action="store_true", help="deploy the newsletter Worker too" + ) parser.add_argument("--app-only", action="store_true", help="deploy the app Worker alone") parser.add_argument("--dry-run", action="store_true", help="deploy nothing, save nothing") parser.add_argument("--wait", type=float, default=WAIT_SECONDS, help="seconds per attempt") @@ -401,8 +454,10 @@ def main( sleep: Callable[[float], None] = time.sleep, ) -> int: args = build_parser().parse_args(argv) - if args.app_only and (args.rss or args.promote): - print("--app-only cannot be combined with --rss or --promote", file=sys.stderr) + if args.app_only and (args.rss or args.promote or args.newsletter): + print( + "--app-only cannot be combined with --rss, --promote or --newsletter", file=sys.stderr + ) return 2 try: provision_trial.load_rendered(args.slug) @@ -410,6 +465,8 @@ def main( commit = step.commit() # Read before anything deploys, so a deploy that cannot be checked never starts. state = provision_trial.wizard_state(args.slug) + if note := provision_trial.newsletter_note(args.slug): + say(note) headers = step.access_headers() dotenv = step.doctor_env(state) digest = step.digest(commit) @@ -417,17 +474,26 @@ def main( rendered = _snapshot(args.slug) app_deployed = False try: + workers = trial_workers(state) + if args.newsletter and "newsletter" not in workers: + raise DeployError( + f".env.trial-{args.slug}-wizard has no {NEWSLETTER_KV_KEY}: this trial has" + " no newsletter Worker. Run the wizard from stage 5 to add one" + ) configs = step.render(state, digest) - forced = {w for w in WORKER_DIRS if getattr(args, w)} + forced = {w for w in workers if getattr(args, w)} baselines = step.baselines(state) changed = step.changed(baselines, commit) extra = workers_to_deploy(changed, forced, args.app_only) - for worker in WORKER_DIRS: + for worker in workers: why = f"deploy ({extra[worker]})" if worker in extra else "unchanged, skipped" say(f"{worker} Worker: {why}") step.sh(["bun", "install", "--frozen-lockfile"]) step.deploy("app") app_deployed = not args.dry_run + if args.dry_run: + # The configs are put back below, so each Worker's dry run reads its own now. + step.deploy_workers(extra) finally: # The configs on disk keep naming the image the trial runs. if not app_deployed: @@ -437,16 +503,12 @@ def main( pinned = { key: baselines[worker] or UNKNOWN for worker, key in BASELINE_KEYS.items() - if key not in state + if worker in workers and key not in state } image = {"IMAGE_TAG": image_tag(commit), "IMAGE_DIGEST": digest} write_state(args.slug, image | pinned) - if "rss" in extra: - step.sh(["bun", "install", "--frozen-lockfile", "--cwd", WORKER_DIRS["rss"]]) - for worker in extra: - step.deploy(worker) - if not args.dry_run: - write_state(args.slug, {BASELINE_KEYS[worker]: commit}) + if not args.dry_run: + step.deploy_workers(extra, commit) if args.dry_run: say("Dry run: nothing was deployed, and the configs and wizard state are unchanged.") return 0 diff --git a/scripts/provision_trial.py b/scripts/provision_trial.py index 6ea00a5d..f1b27401 100644 --- a/scripts/provision_trial.py +++ b/scripts/provision_trial.py @@ -37,6 +37,8 @@ _LABEL = r"[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?" _DOMAIN = rf"^{_LABEL}(\.{_LABEL})+$" SLUG = r"[a-z][a-z0-9]{1,19}" +# The wizard state key whose presence alone says the trial has a newsletter Worker. +NEWSLETTER_KV_KEY = "TRIAL_NEWSLETTER_KV_ID" class DriftError(Exception): @@ -255,13 +257,44 @@ def _names(slug: str, n: dict[str, str]) -> dict[str, str]: } +def newsletter_path(slug: str) -> str: + return f"workers/newsletter/wrangler.trial-{slug}.toml" + + +def has_newsletter(slug: str) -> bool: + """Whether the trial has a newsletter Worker: its wizard state holds the KV id, nothing else.""" + return bool(wizard_state(slug).get(NEWSLETTER_KV_KEY)) + + +def newsletter_note(slug: str) -> str: + """One line for a newsletter config on disk that the wizard state has no KV id for, else "". + + Such a config is never read or validated and nothing is done to it: a trial wired before + the newsletter Worker has no id, and a stray file does not give it one. + """ + if has_newsletter(slug) or not (ROOT / newsletter_path(slug)).exists(): + return "" + return ( + f"{newsletter_path(slug)} exists but .env.trial-{slug}-wizard has no {NEWSLETTER_KV_KEY}:" + " it is left alone, and so is its NEWSLETTERS KV. To give this trial a newsletter Worker," + f" save that KV's id as {NEWSLETTER_KV_KEY} in the wizard state and run this again." + ) + + def rendered_paths(slug: str) -> dict[str, str]: - """Where `render` writes the app, promote and rss configs, relative to the repo root.""" - return { + """Where `render` writes a trial's configs, relative to the repo root. + + The app, promote and rss configs always; the newsletter config only when the wizard + state holds the trial's newsletter KV id and the config has been rendered. + """ + paths = { "app": f"wrangler.trial-{slug}.toml", "promote": f"workers/promote/wrangler.trial-{slug}.toml", "rss": f"workers/rss/wrangler.trial-{slug}.toml", } + if has_newsletter(slug) and (ROOT / newsletter_path(slug)).exists(): + paths["newsletter"] = newsletter_path(slug) + return paths def production_names() -> set[str]: @@ -269,11 +302,15 @@ def production_names() -> set[str]: n = _template_names() d1 = _rss_d1() kv = _get(PROMOTE, _load(PROMOTE), "kv_namespaces", 0, "id") - return {*n.values(), d1["database_name"], d1["database_id"], kv} + newsletter_kv = _get(NEWSLETTER, _load(NEWSLETTER), "kv_namespaces", 0, "id") + return {*n.values(), d1["database_name"], d1["database_id"], kv, newsletter_kv} def load_rendered(slug: str) -> dict[str, dict]: - """A trial's three rendered configs, parsed, once each names only this trial's resources. + """A trial's rendered configs, parsed, once each names only this trial's resources. + + The newsletter config is checked when the wizard state holds the trial's KV id for it, and + left out, unread, when it does not. The scripts that act on a live trial (`reset_trial.py`, `deploy_trial.py`) call this before any wrangler command, so a slug that fails the naming rule, a config @@ -305,15 +342,26 @@ def load_rendered(slug: str) -> dict[str, dict]: f"{rendered_paths(slug)['rss']} name": (out["rss"].get("name"), want["rss"]), f"{rendered_paths(slug)['rss']} database_name": (d1.get("database_name"), want["d1"]), } + ids = [ + (f"{rendered_paths(slug)['rss']} database_id", d1.get("database_id")), + (f"{rendered_paths(slug)['promote']} kv_namespaces[0].id", kv.get("id")), + ] + if "newsletter" in out: + rel = rendered_paths(slug)["newsletter"] + newsletter_kv = (out["newsletter"].get("kv_namespaces") or [{}])[0] + named[f"{rel} name"] = (out["newsletter"].get("name"), want["newsletter"]) + ids.append((f"{rel} kv_namespaces[0].id", newsletter_kv.get("id"))) + if newsletter_kv.get("binding") != "NEWSLETTERS": + raise ProductionNameError( + f"{rel} kv_namespaces[0].binding is {newsletter_kv.get('binding')!r}," + " expected 'NEWSLETTERS'" + ) for label, (got, expected) in named.items(): if got in taken: raise ProductionNameError(f"{label} is {got!r}, a production name") if got != expected: raise ProductionNameError(f"{label} is {got!r}, expected {expected!r}") - for label, got in ( - (f"{rendered_paths(slug)['rss']} database_id", d1.get("database_id")), - (f"{rendered_paths(slug)['promote']} kv_namespaces[0].id", kv.get("id")), - ): + for label, got in ids: if not got or got in taken: raise ProductionNameError(f"{label} is {got!r}, production's or empty") return out @@ -390,8 +438,7 @@ def cmd_render(args, parser) -> int: paths["rss"]: _render_rss(args, n, t), } if args.newsletter_kv_id: - rel = f"workers/newsletter/wrangler.trial-{args.slug}.toml" - outputs[rel] = _render_newsletter(args, n, t) + outputs[newsletter_path(args.slug)] = _render_newsletter(args, n, t) for rel, text in outputs.items(): (ROOT / rel).write_text(text, encoding="utf-8") print(rel) diff --git a/scripts/reset_trial.py b/scripts/reset_trial.py index 0819fb61..57cf2047 100644 --- a/scripts/reset_trial.py +++ b/scripts/reset_trial.py @@ -9,7 +9,13 @@ Cleared, in the trial's D1: every row of the tables listed at the end of this help. In `settings`, only the keys listed there, written back to what -`settings.example.toml` ships. In the vote Worker's KV: every pending vote. +`settings.example.toml` ships. In the vote Worker's KV: every pending vote. In the +newsletter Worker's NEWSLETTERS KV, when the trial has one: every stored issue. Whether +it has one is the wizard state's TRIAL_NEWSLETTER_KV_ID alone. A trial wired before that +Worker has no id, and the reset leaves the KV out. A config on disk with no id is +reported in one line, never read or validated, and its KV left alone; an id with no config +on disk is refused, naming the render command that writes it, because the reset could not +clear what it is told the trial has. Kept: `usage_log` (what the trial has spent), `schema_migrations` (the schema steps already run), every other `settings` key (the LLM provider and model the owner @@ -95,6 +101,9 @@ # workers/promote/src/index.js keys each pending vote under this prefix. VOTE_PREFIX = "promote:" +# workers/newsletter/src/index.js keys each received issue under this prefix. +NEWSLETTER_PREFIX = "nl:" +NEWSLETTER_KV_KEY = provision_trial.NEWSLETTER_KV_KEY class ResetError(Exception): @@ -170,6 +179,41 @@ def _json(stdout: str): raise ResetError(f"wrangler printed no JSON: {stdout.strip()[:200]!r}") +# Each `render` flag, the state key it takes its value from, and the wizard stage that saves it. +RENDER_FLAGS = ( + ("--domain", "DOMAIN", 2), + ("--account-id", "ACCOUNT_ID", 2), + ("--image-digest", "IMAGE_DIGEST", 2), + ("--kv-id", "TRIAL_KV_ID", 5), + ("--newsletter-kv-id", NEWSLETTER_KV_KEY, 5), + ("--d1-id", "TRIAL_D1_ID", 4), +) + + +def _render_hint(slug: str, state: dict[str, str]) -> str: + """The lines that say how to get the config back, each command on a line of its own. + + The `render` command trial-wizard.sh's stage 8 runs, from the state's values. A command with + a hole would not run, and `--from 8` stops on a value no earlier stage of that run saved, so + with a value missing it names each missing key and the stage that writes it. + """ + missing = [(key, stage) for _, key, stage in RENDER_FLAGS if not state.get(key)] + if missing: + saved = "\n".join( + f" {key}: bash scripts/trial-wizard.sh {slug} --from {stage}" for key, stage in missing + ) + keys = ", ".join(key for key, _ in missing) + return ( + f"The state lacks {keys}, so the render command cannot be written yet." + f" Save each with:\n{saved}" + ) + flags = " ".join(f"{flag} {state[key]}" for flag, key, _ in RENDER_FLAGS) + return ( + "Render the configs from the state's values with:\n" + f" uv run python scripts/provision_trial.py render --slug {slug} {flags}" + ) + + class Trial: def __init__(self, slug: str, run: Runner) -> None: try: @@ -187,6 +231,31 @@ def __init__(self, slug: str, run: Runner) -> None: f"refused: {rel} names {got}, but .env.trial-{slug}-wizard has" f" {key}={state.get(key) or '(none)'}: not provably this trial's" ) + self.newsletter_config = None + self.newsletter_binding = "" + self.newsletter_note = "" + saved = state.get(NEWSLETTER_KV_KEY) + if saved: + if "newsletter" not in configs: + raise ResetError( + f"refused: .env.trial-{slug}-wizard has {NEWSLETTER_KV_KEY}={saved}, but" + f" {provision_trial.newsletter_path(slug)} is not there, so its KV cannot be" + f" cleared. {_render_hint(slug, state)}\n" + "Or deploy the trial, which renders them too:\n" + f" uv run python scripts/deploy_trial.py --slug {slug}\n" + "Then run this again" + ) + kv = configs["newsletter"]["kv_namespaces"][0] + if kv["id"] != saved: + raise ResetError( + f"refused: {paths['newsletter']} names {kv['id']}, but" + f" .env.trial-{slug}-wizard has {NEWSLETTER_KV_KEY}={saved}:" + " not provably this trial's" + ) + self.newsletter_config = paths["newsletter"] + self.newsletter_binding = kv["binding"] + else: + self.newsletter_note = provision_trial.newsletter_note(slug) self.rss_config = paths["rss"] self.promote_config = paths["promote"] self.d1 = configs["rss"]["d1_databases"][0]["database_name"] @@ -211,41 +280,48 @@ def d1_query(self, sql: str, *, write: bool = False) -> list[dict]: def rows(self, sql: str) -> list[dict]: return self.d1_query(sql)[0]["results"] - def votes(self) -> list[str]: + def keys(self, config: str, binding: str, prefix: str) -> list[str]: cmd = wrangler( - self.promote_config, - "kv", - "key", - "list", - "--binding", - self.kv_binding, - "--prefix", - VOTE_PREFIX, - "--remote", + config, "kv", "key", "list", "--binding", binding, "--prefix", prefix, "--remote" ) return [k["name"] for k in _json(self._run(cmd))] - def delete_votes(self, keys: list[str]) -> None: + def delete_keys(self, config: str, binding: str, keys: list[str]) -> None: with tempfile.TemporaryDirectory() as tmp: - listing = Path(tmp) / "votes.json" + listing = Path(tmp) / "keys.json" listing.write_text(json.dumps(keys), encoding="utf-8") self._run( wrangler( - self.promote_config, + config, "kv", "bulk", "delete", str(listing), "--binding", - self.kv_binding, + binding, "--remote", "--force", ) ) + def votes(self) -> list[str]: + return self.keys(self.promote_config, self.kv_binding, VOTE_PREFIX) + + def delete_votes(self, keys: list[str]) -> None: + self.delete_keys(self.promote_config, self.kv_binding, keys) + + def newsletters(self) -> list[str]: + """The stored issues; none for a trial that has no newsletter KV.""" + if not self.newsletter_config: + return [] + return self.keys(self.newsletter_config, self.newsletter_binding, NEWSLETTER_PREFIX) -def survey(trial: Trial, say: Callable[[str], None]) -> tuple[list[str], list[str]]: - """Print what the write would touch; return the present tables and the pending votes.""" + def delete_newsletters(self, keys: list[str]) -> None: + self.delete_keys(self.newsletter_config, self.newsletter_binding, keys) + + +def survey(trial: Trial, say: Callable[[str], None]) -> tuple[list[str], list[str], list[str]]: + """Print what the write would touch; return the present tables, pending votes and issues.""" found = {r["name"] for r in trial.rows("SELECT name FROM sqlite_master WHERE type = 'table'")} present = [t for t in CLEARED if t in found] counts = trial.rows(count_sql(present))[0] if present else {} @@ -260,9 +336,17 @@ def survey(trial: Trial, say: Callable[[str], None]) -> tuple[list[str], list[st say(f" {key:<28} {stored.get(key, '(unset)')} -> {json.dumps(defaults[key])}") votes = trial.votes() say(f"KV {trial.kv_binding} ({trial.promote_config}): {len(votes)} pending votes") + issues = trial.newsletters() + if trial.newsletter_config: + say( + f"KV {trial.newsletter_binding} ({trial.newsletter_config}):" + f" {len(issues)} stored newsletter issues" + ) + if trial.newsletter_note: + say(trial.newsletter_note) say("Kept: " + "; ".join(f"{t} ({why})" for t, why in KEPT.items())) say("Kept: the Workers, their secrets, the Access policy, the Pages project and its digests.") - return present, votes + return present, votes, issues def _runner(env: dict[str, str]) -> Runner: @@ -298,30 +382,36 @@ def main( args = build_parser().parse_args(argv) try: trial = Trial(args.slug, run or _runner(wrangler_env(args.slug))) - present, votes = survey(trial, say) + present, votes, issues = survey(trial, say) statements = reset_sql(present, shipped_defaults(), now()) say("SQL the write sends, in one call:") for statement in statements: say(f" {statement};") if votes: say(f"and a KV bulk delete of the {len(votes)} pending votes.") + if issues: + say(f"and a KV bulk delete of the {len(issues)} stored newsletter issues.") if not args.yes: say("Dry run: nothing was written. Run again with --yes to write.") return 0 trial.d1_query("; ".join(statements), write=True) if votes: trial.delete_votes(votes) + if issues: + trial.delete_newsletters(issues) left = trial.rows(count_sql(present))[0] if present else {} still = {t: n for t, n in left.items() if n} listed = trial.votes() + issues_left = trial.newsletters() # WHY a warning, not a failure: both writes succeeded, and a recount can still see # rows. KV listing is eventually consistent, so a deleted vote can list for a while, # and an rss poll already running can buffer articles between delete and recount. - if still or listed: + if still or listed or issues_left: + kept = f" and {len(issues_left)} newsletter issues" if trial.newsletter_config else "" print( f"warning: after the reset, {still or 'no table'} still holds rows and" - f" {len(listed)} votes still list in KV. Run the dry run again in a minute:" - " what stays then is real.", + f" {len(listed)} votes{kept} still list in KV. Run the dry run again in a" + " minute: what stays then is real.", file=sys.stderr, ) say(f"Reset {args.slug}: it now waits for onboarding.") diff --git a/scripts/trial-wizard.sh b/scripts/trial-wizard.sh index be36545e..6034e870 100755 --- a/scripts/trial-wizard.sh +++ b/scripts/trial-wizard.sh @@ -351,6 +351,44 @@ has_newsletter() { return 1 } +# worker_url NAME VAR RC LOG settles the URL to probe after a gated deploy and records how it +# was reached in URL_ORIGIN, which `answered` reads. An unreadable log keeps the URL already +# saved instead of overwriting it with nothing. +worker_url() { + local name="$1" var="$2" rc="$3" log="$4" found="" + URL_ORIGIN="deployed" + if (( rc == 0 )); then + found=$(grep -oE 'https://[a-z0-9.-]+\.workers\.dev' "$log" | head -1 || true) + if [[ -n "$found" ]]; then + printf -v "$var" '%s' "$found" + write_env "$var" "$found" + else + URL_ORIGIN="its URL could not be read from the deploy output in $log" + fi + else + URL_ORIGIN="its deploy was refused here" + fi + ask_match "$var" "$name Worker URL (Enter keeps the one shown):" '^https://[a-z0-9.-]+\.workers\.dev$' + write_env "$var" "${!var}" +} + +# answered URL CODE dies when the probe got no HTTP answer at all (curl prints 000), or a 404 +# from a URL this run's deploy did not print, saying how the URL was reached instead of +# reporting the code as a wrong answer. +answered() { + local got="got no answer" + if [[ "$2" == 404 ]]; then + got="answered 404" + elif [[ "$2" != 000 ]]; then + return 0 + fi + if [[ "$URL_ORIGIN" == deployed ]]; then + [[ "$2" == 000 ]] || return 0 + die "$1 $got, though its deploy printed this URL: it may not be live yet." + fi + die "$1 $got: $URL_ORIGIN, so the Worker may not exist at that URL." +} + bearer_header_file() { local f f=$(mktemp) @@ -767,13 +805,9 @@ stage_14() { gated rss "Deploy the rss Worker with $RSS_CFG?" \ bunx wrangler deploy --config "$RSS_CFG" --env-file /dev/null --secrets-file "$RSS_ENV" || rc=$? [[ $rc -eq 0 || $rc -eq 2 ]] || die "rss Worker deploy failed." - if [[ $rc -eq 0 ]]; then - TRIAL_RSS_URL=$(grep -oE 'https://[a-z0-9.-]+\.workers\.dev' "$LOG_DIR/rss.log" | head -1 || true) - write_env TRIAL_RSS_URL "$TRIAL_RSS_URL" - fi - ask_match TRIAL_RSS_URL "rss Worker URL (Enter keeps the one shown):" '^https://[a-z0-9.-]+\.workers\.dev$' - write_env TRIAL_RSS_URL "$TRIAL_RSS_URL" + worker_url rss TRIAL_RSS_URL "$rc" "$LOG_DIR/rss.log" code=$(curl -s -o /dev/null -w '%{http_code}' --max-time 30 "$TRIAL_RSS_URL/stats" || true) + answered "$TRIAL_RSS_URL/stats" "$code" [[ "$code" == 401 ]] || die "$TRIAL_RSS_URL/stats without the bearer answered $code, not 401." printf ' %s✓%s /stats without the bearer → 401\n' "$GREEN" "$RESET" poll=$(curl -s -X POST --max-time 120 -H "Authorization: Bearer $TRIAL_WORKER_TOKEN" "$TRIAL_RSS_URL/poll" || true) @@ -796,18 +830,15 @@ stage_14() { gated newsletter "Deploy the newsletter Worker with $NEWSLETTER_CFG?" \ bunx wrangler deploy --config "$NEWSLETTER_CFG" --env-file /dev/null --secrets-file "$NEWSLETTER_ENV" || rc=$? [[ $rc -eq 0 || $rc -eq 2 ]] || die "Newsletter Worker deploy failed." - if [[ $rc -eq 0 ]]; then - TRIAL_NEWSLETTER_URL=$(grep -oE 'https://[a-z0-9.-]+\.workers\.dev' "$LOG_DIR/newsletter.log" | head -1 || true) - write_env TRIAL_NEWSLETTER_URL "$TRIAL_NEWSLETTER_URL" - fi - ask_match TRIAL_NEWSLETTER_URL "Newsletter Worker URL (Enter keeps the one shown):" '^https://[a-z0-9.-]+\.workers\.dev$' - write_env TRIAL_NEWSLETTER_URL "$TRIAL_NEWSLETTER_URL" + worker_url newsletter TRIAL_NEWSLETTER_URL "$rc" "$LOG_DIR/newsletter.log" code=$(curl -s -o /dev/null -w '%{http_code}' --max-time 30 "$TRIAL_NEWSLETTER_URL/newsletters" || true) + answered "$TRIAL_NEWSLETTER_URL/newsletters" "$code" [[ "$code" == 401 ]] || die "$TRIAL_NEWSLETTER_URL/newsletters without the bearer answered $code, not 401." printf ' %s✓%s /newsletters without the bearer → 401\n' "$GREEN" "$RESET" hdr=$(bearer_header_file) code=$(curl -s -o /dev/null -w '%{http_code}' --max-time 30 -H "@$hdr" "$TRIAL_NEWSLETTER_URL/newsletters" || true) rm -f "$hdr" + answered "$TRIAL_NEWSLETTER_URL/newsletters" "$code" [[ "$code" == 200 ]] || die "$TRIAL_NEWSLETTER_URL/newsletters with the bearer answered $code, not 200: the secret did not land." printf ' %s✓%s /newsletters with the bearer → 200\n' "$GREEN" "$RESET" fi diff --git a/tests/test_deploy_trial.py b/tests/test_deploy_trial.py index f50ffb73..d54497ea 100644 --- a/tests/test_deploy_trial.py +++ b/tests/test_deploy_trial.py @@ -4,13 +4,14 @@ /api/build answer is a stub. The configs are rendered for real, in a temporary tree. """ +import os import subprocess import tomllib from pathlib import Path import httpx import pytest -from trial_tree import D1, DIGEST, SLUG, load, make_tree +from trial_tree import D1, DIGEST, NEWSLETTER_KV, SLUG, load, make_tree pytestmark = pytest.mark.unit @@ -24,6 +25,7 @@ ACCESS = {"CF-Access-Client-Id": "cid", "CF-Access-Client-Secret": "sek-SECRET"} RSS_CONFIG = f"workers/rss/wrangler.trial-{SLUG}.toml" PROMOTE_CONFIG = f"workers/promote/wrangler.trial-{SLUG}.toml" +NEWSLETTER_CONFIG = f"workers/newsletter/wrangler.trial-{SLUG}.toml" class FakeRunner: @@ -402,3 +404,306 @@ def test_an_access_redirect_is_a_failed_ask(deploy) -> None: with pytest.raises(deploy.DeployError, match="answered 302"): deploy.fetch_build_sha("https://t.example.com", ACCESS, client) + + +# --- the newsletter Worker + + +def _with_access(tree: Path) -> Path: + (tree / f".env.trial-{SLUG}.access").write_text( + "CYRIS_ACCESS_CLIENT_ID=cid\nCYRIS_ACCESS_CLIENT_SECRET=sek-SECRET\n", encoding="utf-8" + ) + (tree / f".env.trial-{SLUG}").write_text(f"CYRIS_STORE_DATABASE_ID={D1}\n", encoding="utf-8") + return tree + + +@pytest.fixture +def newsletter_tree(tmp_path: Path) -> Path: + return _with_access(make_tree(tmp_path, "deploy_trial", newsletter=True)) + + +@pytest.fixture +def newsletter_deploy(newsletter_tree: Path, monkeypatch): + return load(newsletter_tree, "deploy_trial", monkeypatch) + + +def test_a_changed_newsletter_worker_is_installed_and_deployed_with_its_own_config( + newsletter_deploy, newsletter_tree: Path +) -> None: + runner = FakeRunner(changed={"workers/newsletter"}) + + code, *_ = _main(newsletter_deploy, runner) + + assert code == 0 + assert _deployed(runner) == [APP_CONFIG, NEWSLETTER_CONFIG] + assert ["bun", "install", "--frozen-lockfile", "--cwd", "workers/newsletter"] in runner.calls + assert _state(newsletter_tree)["NEWSLETTER_WORKER_COMMIT"] == COMMIT + + +def test_an_unchanged_newsletter_worker_is_skipped(newsletter_deploy, newsletter_tree) -> None: + runner = FakeRunner() + + code, said, *_ = _main(newsletter_deploy, runner) + + assert code == 0 + assert _deployed(runner) == [APP_CONFIG] + assert "newsletter Worker: unchanged, skipped" in said + assert _state(newsletter_tree)["NEWSLETTER_WORKER_COMMIT"] == PROVISIONED + + +def test_newsletter_asks_for_the_worker_and_app_only_leaves_it(newsletter_deploy) -> None: + asked = FakeRunner() + assert _main(newsletter_deploy, asked, "--newsletter")[0] == 0 + assert _deployed(asked) == [APP_CONFIG, NEWSLETTER_CONFIG] + + app_only = FakeRunner(changed={"workers/newsletter"}) + assert _main(newsletter_deploy, app_only, "--app-only")[0] == 0 + assert _deployed(app_only) == [APP_CONFIG] + + +def test_the_newsletter_config_is_rendered_again_from_the_saved_kv_id( + newsletter_deploy, newsletter_tree: Path +) -> None: + (newsletter_tree / NEWSLETTER_CONFIG).unlink() + + code, *_ = _main(newsletter_deploy, FakeRunner(changed={"workers/newsletter"})) + + assert code == 0 + got = tomllib.loads((newsletter_tree / NEWSLETTER_CONFIG).read_text()) + assert got["kv_namespaces"] == [{"binding": "NEWSLETTERS", "id": NEWSLETTER_KV}] + + +def test_a_trial_without_a_newsletter_kv_gets_no_newsletter_worker(deploy, tree: Path) -> None: + runner = FakeRunner(changed={"workers/newsletter"}) + + code, said, *_ = _main(deploy, runner) + + assert code == 0 + assert _deployed(runner) == [APP_CONFIG] + assert not (tree / NEWSLETTER_CONFIG).exists() + assert not any("newsletter" in line for line in said) + assert "NEWSLETTER_WORKER_COMMIT" not in _state(tree) + + +def test_asking_for_a_newsletter_worker_the_trial_lacks_is_refused(deploy, capsys) -> None: + runner = FakeRunner() + + code, *_ = _main(deploy, runner, "--newsletter") + + assert code == 1 + assert "TRIAL_NEWSLETTER_KV_ID" in capsys.readouterr().err + assert runner.deploys() == [] + + +def test_a_newsletter_config_naming_production_is_refused_before_any_call( + newsletter_deploy, newsletter_tree: Path +) -> None: + path = newsletter_tree / NEWSLETTER_CONFIG + path.write_text(path.read_text().replace("cyris-newsletter-alice", "cyris-newsletter")) + runner = FakeRunner() + + code, *_ = _main(newsletter_deploy, runner) + + assert code == 1 + assert runner.calls == [] + + +def test_every_wrangler_call_of_a_newsletter_trial_names_a_trial_config( + newsletter_deploy, +) -> None: + runner = FakeRunner() + + _main(newsletter_deploy, runner, "--rss", "--promote", "--newsletter") + + wrangler = [c for c in runner.calls if c[:2] == ["bunx", "wrangler"]] + assert [c[c.index("--config") + 1] for c in wrangler] == [ + APP_CONFIG, RSS_CONFIG, PROMOTE_CONFIG, NEWSLETTER_CONFIG, + ] # fmt: skip + assert all(c[c.index("--env-file") + 1] == "/dev/null" for c in wrangler) + + +def test_a_dry_run_dry_runs_the_newsletter_worker_and_saves_nothing( + newsletter_deploy, newsletter_tree: Path +) -> None: + state = (newsletter_tree / f".env.trial-{SLUG}-wizard").read_text() + runner = FakeRunner() + + code, *_ = _main(newsletter_deploy, runner, "--dry-run", "--newsletter") + + assert code == 0 + assert _deployed(runner) == [APP_CONFIG, NEWSLETTER_CONFIG] + assert [c[-1] for c in runner.deploys()] == ["--dry-run", "--dry-run"] + assert (newsletter_tree / f".env.trial-{SLUG}-wizard").read_text() == state + + +def test_the_snapshot_and_restore_cover_the_newsletter_config( + newsletter_deploy, newsletter_tree: Path +) -> None: + path = newsletter_tree / NEWSLETTER_CONFIG + before = path.read_bytes() + + snapshot = newsletter_deploy._snapshot(SLUG) + path.write_text("name = 'changed'\n") + newsletter_deploy._restore(snapshot) + + assert NEWSLETTER_CONFIG in snapshot + assert path.read_bytes() == before + + +def test_a_failed_deploy_removes_a_newsletter_config_this_run_rendered( + newsletter_deploy, newsletter_tree: Path +) -> None: + path = newsletter_tree / NEWSLETTER_CONFIG + path.unlink() + + class Failing(FakeRunner): + def fails(self, cmd): + return True + + code, *_ = _main(newsletter_deploy, Failing()) + + assert code == 1 + assert not path.exists() + + +# --- the state's id alone says whether the trial has a newsletter Worker + + +class ConfigChecking(FakeRunner): + """A wrangler deploy of a config that is not on disk fails, as wrangler's does.""" + + def __init__(self, tree: Path, **kwargs) -> None: + super().__init__(**kwargs) + self.tree = tree + + def fails(self, cmd): + return not (self.tree / cmd[cmd.index("--config") + 1]).exists() + + +def test_a_dry_run_of_a_trial_whose_config_is_missing_renders_it_and_leaves_none( + newsletter_deploy, newsletter_tree: Path +) -> None: + (newsletter_tree / NEWSLETTER_CONFIG).unlink() + state = (newsletter_tree / f".env.trial-{SLUG}-wizard").read_text() + runner = ConfigChecking(newsletter_tree) + + code, said, *_ = _main(newsletter_deploy, runner, "--dry-run", "--newsletter") + + assert code == 0, said + assert _deployed(runner) == [APP_CONFIG, NEWSLETTER_CONFIG] + assert not (newsletter_tree / NEWSLETTER_CONFIG).exists() + assert (newsletter_tree / f".env.trial-{SLUG}-wizard").read_text() == state + + +def test_a_dry_run_reads_each_worker_config_as_rendered_this_run( + newsletter_deploy, newsletter_tree: Path +) -> None: + path = newsletter_tree / NEWSLETTER_CONFIG + path.write_text(path.read_text() + "# stale\n") + seen: list[str] = [] + + class Reading(FakeRunner): + def fails(self, cmd): + if NEWSLETTER_CONFIG in cmd: + seen.append((newsletter_tree / NEWSLETTER_CONFIG).read_text()) + return False + + _main(newsletter_deploy, Reading(), "--dry-run", "--newsletter") + + assert seen and "# stale" not in seen[0] + assert "# stale" in path.read_text() + + +def test_a_config_with_no_saved_id_is_not_a_newsletter_worker( + newsletter_deploy, newsletter_tree: Path, capsys +) -> None: + state = newsletter_tree / f".env.trial-{SLUG}-wizard" + state.write_text( + "".join(ln + "\n" for ln in state.read_text().splitlines() if "NEWSLETTER_KV" not in ln) + ) + runner = FakeRunner(changed={"workers/newsletter"}) + + code, said, *_ = _main(newsletter_deploy, runner) + + assert code == 0 + assert _deployed(runner) == [APP_CONFIG] + assert "NEWSLETTER_WORKER_COMMIT" not in _state(newsletter_tree) + assert _main(newsletter_deploy, FakeRunner(), "--newsletter")[0] == 1 + assert "TRIAL_NEWSLETTER_KV_ID" in capsys.readouterr().err + + +@pytest.mark.parametrize( + "stray", + [ + pytest.param(lambda t: t.replace("cyris-newsletter-alice", "cyris-newsletter"), id="prod"), + pytest.param(lambda t: "this is [not toml", id="unparseable"), + ], +) +@pytest.mark.parametrize("flags", [(), ("--dry-run",)], ids=["deploy", "dry-run"]) +def test_a_config_with_no_saved_id_is_reported_and_never_read_or_validated( + newsletter_deploy, newsletter_tree: Path, stray, flags +) -> None: + state = newsletter_tree / f".env.trial-{SLUG}-wizard" + state.write_text( + "".join(ln + "\n" for ln in state.read_text().splitlines() if "NEWSLETTER_KV" not in ln) + ) + path = newsletter_tree / NEWSLETTER_CONFIG + path.write_text(stray(path.read_text())) + before = path.read_bytes() + runner = FakeRunner(changed={"workers/newsletter"}) + + code, said, *_ = _main(newsletter_deploy, runner, *flags) + + assert code == 0, said + notes = [line for line in said if "TRIAL_NEWSLETTER_KV_ID" in line] + assert notes == [newsletter_deploy.provision_trial.newsletter_note(SLUG)] + assert notes[0].count(NEWSLETTER_CONFIG) == 1 and "left alone" in notes[0] + assert not any(NEWSLETTER_CONFIG in c for c in runner.calls) + assert path.read_bytes() == before + + +def _without_newsletter_id(tree: Path) -> Path: + state = tree / f".env.trial-{SLUG}-wizard" + state.write_text( + "".join(ln + "\n" for ln in state.read_text().splitlines() if "NEWSLETTER_KV" not in ln) + ) + return tree / NEWSLETTER_CONFIG + + +@pytest.mark.parametrize("flags", [(), ("--dry-run",)], ids=["deploy", "dry-run"]) +def test_a_config_with_no_saved_id_is_never_opened_or_written( + newsletter_deploy, newsletter_tree: Path, monkeypatch, flags +) -> None: + path = _without_newsletter_id(newsletter_tree) + os.utime(path, ns=(10**18, 10**18)) + before = path.stat().st_mtime_ns + target = path.resolve() + touched: list[str] = [] + + def probe(name: str) -> None: + real = getattr(Path, name) + + def wrapper(self, *args, **kwargs): + if self.resolve() == target: + touched.append(name) + return real(self, *args, **kwargs) + + monkeypatch.setattr(Path, name, wrapper) + + for name in ("open", "read_text", "read_bytes", "write_text", "write_bytes", "unlink", "touch"): + probe(name) + + code, said, *_ = _main(newsletter_deploy, FakeRunner(changed={"workers/newsletter"}), *flags) + + assert code == 0, said + assert touched == [] + assert path.stat().st_mtime_ns == before + + +@pytest.mark.parametrize("flags", [(), ("--dry-run",)], ids=["deploy", "dry-run"]) +def test_a_trial_with_a_saved_id_gets_no_stray_config_note(newsletter_deploy, flags) -> None: + code, said, *_ = _main(newsletter_deploy, FakeRunner(), *flags) + + assert code == 0, said + assert newsletter_deploy.provision_trial.newsletter_note(SLUG) == "" + assert not any("TRIAL_NEWSLETTER_KV_ID" in line or "left alone" in line for line in said) diff --git a/tests/test_provision_trial.py b/tests/test_provision_trial.py index 4b9a5be9..da9d4432 100644 --- a/tests/test_provision_trial.py +++ b/tests/test_provision_trial.py @@ -8,6 +8,7 @@ from pathlib import Path import pytest +from trial_tree import SLUG, load, make_tree pytestmark = [pytest.mark.unit, pytest.mark.guard] @@ -652,3 +653,59 @@ def test_a_root_only_pattern_misses_the_worker_directories(tmp_path: Path) -> No repo = _repo_with_gitignore(tmp_path, "/wrangler.trial-*.toml") assert [_ignored(repo, p) for p in OUTPUTS] == [0, 1, 1, 1] + + +# --- the newsletter config in rendered_paths and load_rendered + +TEMPLATE_NEWSLETTER_KV = "eb4ebc842d024684aa4223936ec8e4d1" + + +@pytest.fixture +def rendered(tmp_path: Path, monkeypatch): + def make(*, newsletter: bool): + tree = make_tree(tmp_path, newsletter=newsletter) + return tree, load(tree, "provision_trial", monkeypatch) + + return make + + +def test_rendered_paths_lists_the_newsletter_config_only_when_the_trial_has_one(rendered) -> None: + _, with_one = rendered(newsletter=True) + + assert with_one.rendered_paths(SLUG)["newsletter"] == OUT_NEWSLETTER + assert set(with_one.load_rendered(SLUG)) == {"app", "promote", "rss", "newsletter"} + + +def test_a_trial_without_a_newsletter_config_loads_as_before(rendered) -> None: + _, old = rendered(newsletter=False) + + assert set(old.rendered_paths(SLUG)) == {"app", "promote", "rss"} + assert set(old.load_rendered(SLUG)) == {"app", "promote", "rss"} + + +@pytest.mark.parametrize( + "old,new,match", + [ + ('name = "cyris-newsletter-alice"', 'name = "cyris-newsletter"', "a production name"), + ('name = "cyris-newsletter-alice"', 'name = "cyris-newsletter-bob"', "expected"), + (NL_KV, TEMPLATE_NEWSLETTER_KV, "production's or empty"), + ('binding = "NEWSLETTERS"', 'binding = "OTHER"', "binding"), + ], +) +def test_a_newsletter_config_naming_production_or_drifted_is_refused( + rendered, old: str, new: str, match: str +) -> None: + tree, mod = rendered(newsletter=True) + path = tree / OUT_NEWSLETTER + text = path.read_text(encoding="utf-8") + assert text.count(old) == 1 + path.write_text(text.replace(old, new), encoding="utf-8") + + with pytest.raises(mod.ProductionNameError, match=match): + mod.load_rendered(SLUG) + + +def test_production_names_include_the_newsletter_templates_kv_id(rendered) -> None: + _, mod = rendered(newsletter=False) + + assert TEMPLATE_NEWSLETTER_KV in mod.production_names() diff --git a/tests/test_reset_trial.py b/tests/test_reset_trial.py index ec90bece..37896434 100644 --- a/tests/test_reset_trial.py +++ b/tests/test_reset_trial.py @@ -7,18 +7,27 @@ import json import re +import shlex import sqlite3 +import subprocess +import sys import tomllib from pathlib import Path import pytest from fakes import CompoundSelectLimitedD1 -from trial_tree import ROOT, SLUG, load, make_tree +from trial_tree import ( + ROOT, + SLUG, + load, + make_tree, +) pytestmark = pytest.mark.unit RSS_CONFIG = f"workers/rss/wrangler.trial-{SLUG}.toml" PROMOTE_CONFIG = f"workers/promote/wrangler.trial-{SLUG}.toml" +NEWSLETTER_CONFIG = f"workers/newsletter/wrangler.trial-{SLUG}.toml" SCHEMA = ROOT / "src/cyris/adapters/store/schema.sql" RSS_WORKER = ROOT / "workers/rss/src/index.js" LLM = {"llm_provider.provider": "gemini", "llm_provider.model": "gemini-2.5-flash"} @@ -48,6 +57,7 @@ def __init__(self) -> None: for statement in _rss_ddl(): self.db.execute(statement) self.kv = {"promote:aa": "{}", "promote:bb": "{}", "other:cc": "{}"} + self.newsletter_kv = {"nl:aa": "{}", "nl:bb": "{}", "other:zz": "{}"} self.calls: list[list[str]] = [] self.seed() @@ -89,12 +99,15 @@ def __call__(self, cmd: list[str]) -> str: return json.dumps([{"results": [], "success": True}]) rows = [dict(r) for r in self.db.execute(sql)] return "banner line\n" + json.dumps([{"results": rows, "success": True}]) - if cmd[2:5] == ["kv", "key", "list"]: - prefix = cmd[cmd.index("--prefix") + 1] - return json.dumps([{"name": k} for k in self.kv if k.startswith(prefix)]) - if cmd[2:5] == ["kv", "bulk", "delete"]: + if cmd[2:4] == ["kv", "key"] or cmd[2:5] == ["kv", "bulk", "delete"]: + store = {"PROMOTIONS": self.kv, "NEWSLETTERS": self.newsletter_kv}[ + cmd[cmd.index("--binding") + 1] + ] + if cmd[4] == "list": + prefix = cmd[cmd.index("--prefix") + 1] + return json.dumps([{"name": k} for k in store if k.startswith(prefix)]) for key in json.loads(Path(cmd[5]).read_text(encoding="utf-8")): - del self.kv[key] + del store[key] return "" raise AssertionError(f"unexpected command {cmd}") @@ -395,3 +408,247 @@ def test_the_vote_prefix_is_the_promote_workers(reset) -> None: worker = (ROOT / "workers/promote/src/index.js").read_text(encoding="utf-8") assert f'prefix: "{reset.VOTE_PREFIX}"' in worker + + +# --- the newsletter KV + + +@pytest.fixture +def newsletter_tree(tmp_path: Path) -> Path: + return make_tree(tmp_path, "reset_trial", newsletter=True) + + +@pytest.fixture +def newsletter_reset(newsletter_tree: Path, monkeypatch): + return load(newsletter_tree, "reset_trial", monkeypatch) + + +def test_the_dry_run_counts_the_newsletter_issues_and_clears_none(newsletter_reset) -> None: + wrangler = FakeWrangler() + + code, out = _main(newsletter_reset, wrangler) + + assert code == 0 + assert any("KV NEWSLETTERS" in line and "2 stored newsletter issues" in line for line in out) + assert wrangler.writes() == [] + assert set(wrangler.newsletter_kv) == {"nl:aa", "nl:bb", "other:zz"} + + +def test_yes_clears_the_trials_newsletter_issues_from_its_own_kv(newsletter_reset) -> None: + wrangler = FakeWrangler() + + code, out = _main(newsletter_reset, wrangler, "--yes") + + assert code == 0, out + assert set(wrangler.newsletter_kv) == {"other:zz"} + assert set(wrangler.kv) == {"other:cc"} + kv = [c for c in wrangler.calls if c[2] == "kv"] + configs = {c[c.index("--config") + 1] for c in kv if "NEWSLETTERS" in c} + assert configs == {NEWSLETTER_CONFIG} + assert all(c[c.index("--env-file") + 1] == "/dev/null" and "--remote" in c for c in kv) + + +def test_no_stored_issue_means_no_newsletter_delete(newsletter_reset) -> None: + wrangler = FakeWrangler() + wrangler.newsletter_kv = {} + + _main(newsletter_reset, wrangler, "--yes") + + assert not any("NEWSLETTERS" in c for c in wrangler.writes()) + + +def test_a_trial_without_a_newsletter_kv_never_touches_one(reset) -> None: + wrangler = FakeWrangler() + + code, out = _main(reset, wrangler, "--yes") + + assert code == 0 + assert not any( + "NEWSLETTERS" in " ".join(c) or "newsletter" in " ".join(c) for c in wrangler.calls + ) + assert set(wrangler.newsletter_kv) == {"nl:aa", "nl:bb", "other:zz"} + assert not any("newsletter" in line for line in out) + + +def test_a_newsletter_kv_id_that_differs_from_the_wizard_state_is_refused( + newsletter_reset, newsletter_tree: Path, capsys +) -> None: + state = newsletter_tree / f".env.trial-{SLUG}-wizard" + lines = [ln for ln in state.read_text().splitlines() if "NEWSLETTER_KV" not in ln] + state.write_text("\n".join([*lines, "TRIAL_NEWSLETTER_KV_ID=" + "0" * 32]) + "\n") + wrangler = FakeWrangler() + + assert newsletter_reset.main(["--slug", SLUG, "--yes"], run=wrangler, say=print) == 1 + assert "TRIAL_NEWSLETTER_KV_ID=" in capsys.readouterr().err + assert wrangler.calls == [] + + +def test_a_newsletter_config_naming_production_is_refused_before_any_call( + newsletter_reset, newsletter_tree: Path, capsys +) -> None: + path = newsletter_tree / NEWSLETTER_CONFIG + path.write_text(path.read_text().replace("cyris-newsletter-alice", "cyris-newsletter")) + wrangler = FakeWrangler() + + assert newsletter_reset.main(["--slug", SLUG, "--yes"], run=wrangler, say=print) == 1 + assert "production name" in capsys.readouterr().err + assert wrangler.calls == [] + + +# --- the state's id alone says whether the trial has a newsletter Worker + + +def _without_newsletter_id(tree: Path) -> None: + state = tree / f".env.trial-{SLUG}-wizard" + lines = [ln for ln in state.read_text().splitlines() if "NEWSLETTER_KV" not in ln] + state.write_text("\n".join(lines) + "\n") + + +def test_an_id_with_no_config_on_disk_is_refused_and_claims_no_clear( + newsletter_reset, newsletter_tree: Path, capsys +) -> None: + (newsletter_tree / NEWSLETTER_CONFIG).unlink() + wrangler = FakeWrangler() + said: list[str] = [] + + code = newsletter_reset.main(["--slug", SLUG, "--yes"], run=wrangler, say=said.append) + + assert code == 1 + assert wrangler.calls == [] + assert not any(line.startswith("Reset") for line in said) + err = capsys.readouterr().err + assert "TRIAL_NEWSLETTER_KV_ID" in err and NEWSLETTER_CONFIG in err + assert set(wrangler.newsletter_kv) == {"nl:aa", "nl:bb", "other:zz"} + + +def test_a_config_with_no_saved_id_is_reported_and_its_kv_left_alone( + newsletter_reset, newsletter_tree: Path +) -> None: + _without_newsletter_id(newsletter_tree) + wrangler = FakeWrangler() + + code, out = _main(newsletter_reset, wrangler, "--yes") + + assert code == 0, out + assert {t: wrangler.count(t) for t in newsletter_reset.CLEARED} == dict.fromkeys( + newsletter_reset.CLEARED, 0 + ) + assert set(wrangler.kv) == {"other:cc"} + assert set(wrangler.newsletter_kv) == {"nl:aa", "nl:bb", "other:zz"} + assert not any("NEWSLETTERS" in " ".join(c) for c in wrangler.calls) + assert not any(NEWSLETTER_CONFIG in c for c in wrangler.calls) + notes = [line for line in out if "TRIAL_NEWSLETTER_KV_ID" in line] + assert len(notes) == 1 + assert NEWSLETTER_CONFIG in notes[0] and "left alone" in notes[0] + assert "save" in notes[0] and "wizard state" in notes[0] + assert out[-1] == f"Reset {SLUG}: it now waits for onboarding." + + +def test_a_config_with_no_saved_id_is_reported_by_the_dry_run_too( + newsletter_reset, newsletter_tree: Path +) -> None: + _without_newsletter_id(newsletter_tree) + wrangler = FakeWrangler() + + code, out = _main(newsletter_reset, wrangler) + + assert code == 0 + assert len([line for line in out if "TRIAL_NEWSLETTER_KV_ID" in line]) == 1 + assert wrangler.writes() == [] + + +@pytest.mark.parametrize( + "stray", + [ + pytest.param(lambda t: t.replace("cyris-newsletter-alice", "cyris-newsletter"), id="prod"), + pytest.param(lambda t: "this is [not toml", id="unparseable"), + ], +) +def test_a_config_with_no_saved_id_is_reported_and_never_read_or_validated( + newsletter_reset, newsletter_tree: Path, stray +) -> None: + _without_newsletter_id(newsletter_tree) + path = newsletter_tree / NEWSLETTER_CONFIG + path.write_text(stray(path.read_text())) + before = path.read_bytes() + wrangler = FakeWrangler() + + code, out = _main(newsletter_reset, wrangler, "--yes") + + assert code == 0, out + notes = [line for line in out if "TRIAL_NEWSLETTER_KV_ID" in line] + assert len(notes) == 1 and NEWSLETTER_CONFIG in notes[0] + assert not any(NEWSLETTER_CONFIG in c for c in wrangler.calls) + assert path.read_bytes() == before + assert set(wrangler.newsletter_kv) == {"nl:aa", "nl:bb", "other:zz"} + + +def _refusal(reset, tree: Path, capsys, *, drop: tuple[str, ...] = ()) -> list[str]: + (tree / NEWSLETTER_CONFIG).unlink() + state = tree / f".env.trial-{SLUG}-wizard" + state.write_text( + "".join( + ln + "\n" for ln in state.read_text().splitlines() if ln.split("=", 1)[0] not in drop + ) + ) + + assert reset.main(["--slug", SLUG, "--yes"], run=FakeWrangler(), say=print) == 1 + + return capsys.readouterr().err.splitlines() + + +def test_the_refusal_for_a_missing_config_prints_a_render_command_that_runs( + newsletter_reset, newsletter_tree: Path, capsys +) -> None: + lines = _refusal(newsletter_reset, newsletter_tree, capsys) + + line = next(ln for ln in lines if "provision_trial.py render" in ln) + tokens = shlex.split(line[line.index("uv run") :]) + assert tokens[:4] == ["uv", "run", "python", "scripts/provision_trial.py"] + done = subprocess.run( + [sys.executable, *tokens[3:]], cwd=newsletter_tree, capture_output=True, text=True + ) + assert done.returncode == 0, done.stderr + assert (newsletter_tree / NEWSLETTER_CONFIG).exists() + assert line.startswith(" uv run") and not line.endswith(".") + wrangler = FakeWrangler() + assert newsletter_reset.main(["--slug", SLUG, "--yes"], run=wrangler, say=print) == 0 + assert set(wrangler.newsletter_kv) == {"other:zz"} + + +def test_the_refusal_names_the_deploy_as_a_command_on_its_own_line( + newsletter_reset, newsletter_tree: Path, capsys +) -> None: + lines = _refusal(newsletter_reset, newsletter_tree, capsys) + + assert f" uv run python scripts/deploy_trial.py --slug {SLUG}" in lines + assert not any(ln.rstrip().endswith((".", ",", ";")) and "uv run" in ln for ln in lines) + + +@pytest.mark.parametrize( + "drop,stage", + [ + (("DOMAIN",), 2), + (("IMAGE_DIGEST",), 2), + (("ACCOUNT_ID", "IMAGE_DIGEST"), 2), + ], + ids=["domain", "digest", "account-and-digest"], +) +def test_the_refusal_names_each_missing_state_key_and_the_stage_that_saves_it( + newsletter_reset, newsletter_tree: Path, capsys, drop, stage +) -> None: + lines = _refusal(newsletter_reset, newsletter_tree, capsys, drop=drop) + + err = "\n".join(lines) + for key in drop: + assert f" {key}: bash scripts/trial-wizard.sh {SLUG} --from {stage}" in lines + assert "--from 8" not in err + assert "provision_trial.py render" not in err + assert f"uv run python scripts/deploy_trial.py --slug {SLUG}" in err + + +def test_a_trial_with_a_saved_id_gets_no_stray_config_note(newsletter_reset) -> None: + code, out = _main(newsletter_reset, FakeWrangler()) + + assert code == 0, out + assert not any("TRIAL_NEWSLETTER_KV_ID" in line or "left alone" in line for line in out) diff --git a/tests/test_trial_wizard.py b/tests/test_trial_wizard.py index 9e433354..210a2e0c 100644 --- a/tests/test_trial_wizard.py +++ b/tests/test_trial_wizard.py @@ -83,7 +83,8 @@ [[ -n "$out" ]] && printf '%s' "${FAKE_SETTINGS_BODY:-}" > "$out" echo "${FAKE_SETTINGS_CODE:-302}" ;; *"/newsletters"*) - if [[ $bearer == yes ]]; then echo "${FAKE_NEWSLETTERS_CODE:-200}" + if [[ -n "${FAKE_NEWSLETTERS_DOWN:-}" ]]; then echo 000 # curl's word for no answer + elif [[ $bearer == yes ]]; then echo "${FAKE_NEWSLETTERS_CODE:-200}" elif [[ -n "${FAKE_NEWSLETTERS_OPEN:-}" ]]; then echo 200 else echo 401; fi ;; *"pages.dev"*) echo 200 ;; @@ -110,7 +111,8 @@ *" --dry-run "*) ;; *" deploy "*) case "$*" in - *"workers/newsletter/"*) echo "https://n-t1.x.workers.dev" ;; + *"workers/newsletter/"*) + [[ -n "${FAKE_NEWSLETTER_NO_URL:-}" ]] || echo "https://n-t1.x.workers.dev" ;; *"workers/rss/"*) echo "https://r-t1.x.workers.dev" ;; *) echo "https://w-t1.x.workers.dev" ;; esac ;; @@ -494,6 +496,70 @@ def test_a_declined_newsletter_deploy_keeps_the_url_and_still_probes(rig) -> Non assert "TRIAL_NEWSLETTER_URL=https://n-t1.x.workers.dev" in run.state.splitlines() +def test_a_refused_newsletter_deploy_with_a_dead_url_says_the_deploy_was_refused(rig) -> None: + r = rig(_stages(13), FAKE_POLL_FEEDS="1", FAKE_NEWSLETTERS_DOWN="1") + _seed_rss_token(r) + + run = r.run(stdin=START + "y\n\nn\n\n" + TAIL) + + assert run.code == 1 + assert "https://n-t1.x.workers.dev/newsletters got no answer" in run.out + assert "its deploy was refused here" in run.out + assert "answered 000" not in run.out + + +def test_an_unreadable_newsletter_url_says_so_and_keeps_the_saved_one(rig) -> None: + r = rig(_stages(13), FAKE_POLL_FEEDS="1", FAKE_NEWSLETTERS_DOWN="1", FAKE_NEWSLETTER_NO_URL="1") + _seed_rss_token(r) + + run = r.run(stdin=START + "y\n\ny\nhttps://n-t1.x.workers.dev\n" + TAIL) + + assert run.code == 1 + assert "its URL could not be read from the deploy output" in run.out + assert "answered 000" not in run.out + assert "TRIAL_NEWSLETTER_URL=https://n-t1.x.workers.dev" in run.state.splitlines() + + +def test_a_newsletter_url_the_deploy_printed_that_does_not_answer_says_so(rig) -> None: + r = rig(_stages(13), FAKE_POLL_FEEDS="1", FAKE_NEWSLETTERS_DOWN="1") + _seed_rss_token(r) + + run = r.run(stdin=START + "y\n\ny\n\n" + TAIL) + + assert run.code == 1 + assert "though its deploy printed this URL: it may not be live yet" in run.out + assert "answered 000" not in run.out + + +def test_a_refused_rss_deploy_with_a_dead_url_says_the_deploy_was_refused(rig) -> None: + r = rig(_stages(13), FAKE_STATS_CODE="000") + + run = r.run(stdin=_answers("n", RSS_URL)) + + assert run.code == 1 + assert f"{RSS_URL}/stats got no answer: its deploy was refused here" in run.out + assert "answered 000" not in run.out + + +def test_a_refused_rss_deploy_answering_404_says_the_deploy_was_refused(rig) -> None: + r = rig(_stages(13), FAKE_STATS_CODE="404") + + run = r.run(stdin=_answers("n", RSS_URL)) + + assert run.code == 1 + assert f"{RSS_URL}/stats answered 404: its deploy was refused here" in run.out + assert "not 401" not in run.out + + +def test_a_404_from_a_url_the_deploy_printed_still_says_what_it_expected(rig) -> None: + r = rig(_stages(13), FAKE_STATS_CODE="404") + + run = r.run(stdin=_answers("y", "")) + + assert run.code == 1 + assert "https://r-t1.x.workers.dev/stats without the bearer answered 404, not 401" in run.out + + def test_the_names_fake_keys_match_what_provision_trial_prints() -> None: real = subprocess.run( [ diff --git a/tests/trial_tree.py b/tests/trial_tree.py index bd01a275..42842f85 100644 --- a/tests/trial_tree.py +++ b/tests/trial_tree.py @@ -17,6 +17,7 @@ ACCT = "0123456789abcdef0123456789abcdef" DIGEST = "sha256:" + "a" * 64 KV = "fedcba9876543210fedcba9876543210" +NEWSLETTER_KV = "1234567890abcdef1234567890abcdef" D1 = "11111111-2222-4333-8444-555555555555" TEMPLATES = [ "wrangler.toml", @@ -26,7 +27,8 @@ ] -def make_tree(tmp_path: Path, *scripts: str) -> Path: +def make_tree(tmp_path: Path, *scripts: str, newsletter: bool = False) -> Path: + """The tree with one trial rendered; `newsletter` gives it the newsletter Worker too.""" tree = tmp_path / "tree" for rel in [ "scripts/provision_trial.py", @@ -38,16 +40,17 @@ def make_tree(tmp_path: Path, *scripts: str) -> Path: (tree / rel).parent.mkdir(parents=True, exist_ok=True) shutil.copy(ROOT / rel, tree / rel) (tree / "scripts/derive-wrangler-config.sh").chmod(0o755) - render(tree, DIGEST) + render(tree, DIGEST, newsletter=newsletter) + ids = f"TRIAL_NEWSLETTER_KV_ID={NEWSLETTER_KV}\n" if newsletter else "" (tree / f".env.trial-{SLUG}-wizard").write_text( - f"DOMAIN=example.com\nACCOUNT_ID={ACCT}\nTRIAL_KV_ID={KV}\nTRIAL_D1_ID={D1}\n" + f"DOMAIN=example.com\nACCOUNT_ID={ACCT}\nTRIAL_KV_ID={KV}\n{ids}TRIAL_D1_ID={D1}\n" f"IMAGE_TAG=image/1111111\nIMAGE_DIGEST={DIGEST}\n", encoding="utf-8", ) return tree -def render(tree: Path, digest: str) -> None: +def render(tree: Path, digest: str, *, newsletter: bool = False) -> None: subprocess.run( [ sys.executable, @@ -63,6 +66,7 @@ def render(tree: Path, digest: str) -> None: digest, "--kv-id", KV, + *(["--newsletter-kv-id", NEWSLETTER_KV] if newsletter else []), "--d1-id", D1, ],