Repository navigation
feat(trial): doctor after deploy, and the newsletter Worker on every path - #166
Merged
Merged
Conversation
The dry run skipped `.env.trial-<slug>.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 <noreply@anthropic.com>
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-<slug> 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-<slug> 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 <noreply@anthropic.com>
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 "<uuid>." 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 <noreply@anthropic.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
|
Reviewed the script changes ( Checked and found sound:
Two small things, neither blocking:
🤖 Generated with Claude Code |
Owner
Author
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
deploy_trial.pyrunscyris doctorafter the build matches, checking doctor's D1 id the way cyris reads it (first line wins), and a dry run makes the same checks of the config and the service token file as a real run.TRIAL_NEWSLETTER_KV_IDalone says whether a trial has one. Provision validates its config, deploy re-renders and deploys it, and reset clears its NEWSLETTERS KV. A stray config with no saved id is reported in one line and never read or written.Ticket: trial-newsletter-release-path (vault pm/cyris/tasks).
Verification
scripts/check.shpassed outside the sandbox at each commit (7f8c8e8: 5320 passed; f8cefd8: 5344 passed; the tip's tree equals the reviewed b490a7e8, which passed).After merge
🤖 Generated with Claude Code